diff --git a/openhcl/underhill_core/src/dispatch/vtl2_settings_worker.rs b/openhcl/underhill_core/src/dispatch/vtl2_settings_worker.rs index b4e5be3d0e..9654656c11 100644 --- a/openhcl/underhill_core/src/dispatch/vtl2_settings_worker.rs +++ b/openhcl/underhill_core/src/dispatch/vtl2_settings_worker.rs @@ -1145,6 +1145,7 @@ fn make_disk_config_inner( scsi_disk_size_in_bytes: disk_params.scsi_disk_size_in_bytes, odx: disk_params.odx, unmap: disk_params.unmap, + get_lba_status: true, max_transfer_length: disk_params.max_transfer_length, optimal_unmap_sectors: None, // TODO }) diff --git a/vm/devices/storage/disk_backend/src/lib.rs b/vm/devices/storage/disk_backend/src/lib.rs index fdb30ca1e0..e085e23724 100644 --- a/vm/devices/storage/disk_backend/src/lib.rs +++ b/vm/devices/storage/disk_backend/src/lib.rs @@ -121,11 +121,6 @@ pub trait DiskIo: 'static + Send + Sync + Inspect { None:: } - /// Optionally returns a trait object to issue get LBA status requests. - fn lba_status(&self) -> Option<&dyn GetLbaStatus> { - None - } - /// Optionally returns a trait object to issue persistent reservation /// requests. fn pr(&self) -> Option<&dyn pr::PersistentReservation> { @@ -182,7 +177,6 @@ impl Disk { fn inspect_extra(&self, resp: &mut inspect::Response<'_>) { resp.field("disk_type", self.0.disk.disk_type()) .field("sector_count", self.0.disk.sector_count()) - .field("supports_lba_status", self.0.disk.lba_status().is_some()) .field("supports_pr", self.0.disk.pr().is_some()) .field( "optimal_unmap_sectors", @@ -302,11 +296,6 @@ impl Disk { } } - /// Optionally returns a trait object to issue get LBA status requests. - pub fn lba_status(&self) -> Option<&dyn GetLbaStatus> { - self.0.disk.lba_status() - } - /// Optionally returns a trait object to issue persistent reservation /// requests. pub fn pr(&self) -> Option<&dyn pr::PersistentReservation> { @@ -376,74 +365,6 @@ impl Unmap for DiskUnmap<'_> { } } -/// A trait to get LBA status and block index information. -pub trait GetLbaStatus { - /// Returns the block index information for the given file offset. - fn file_offset_to_device_block_index_and_length( - &self, - disk: &Disk, - _start_offset: u64, - _get_lba_status_range_length: u64, - _block_size: u64, - ) -> DeviceBlockIndexInfo { - let sector_size = disk.sector_size() as u64; - let sector_count = disk.sector_count(); - let disk_size = sector_size * sector_count; - - // Treat fully allocation disk or fixed disk as one large block and just return - // enough descriptors from the LBA requested till the last LBA on disk. - // - // LbaPerBlock is a ULONG and technically with MAXULONG * 512 byte sectors, - // we can get upto 1.99 TB. The LBA descriptor also holds a ULONG - // LogicalBlockCount and can have an issue for larger than 2TB disks. - let lba_per_block = std::cmp::min(sector_count, u32::MAX.into()); - let block_size_large = lba_per_block * sector_size; - let block_count = ((disk_size + block_size_large - 1) / block_size_large) as u32; - DeviceBlockIndexInfo { - first_partial_block_size: 0, - first_full_block_index: 0, - block_count, - last_partial_block_size: 0, - lba_per_block, - } - } - - /// Returns the LBA status for the given block number. - fn get_block_lba_status( - &self, - _block_number: u32, - _leaf_node_state_only: bool, - ) -> Result { - Ok(LbaStatus::Mapped) - } -} - -/// The LBA status of a block. -#[derive(Debug, Copy, Clone, PartialEq, Eq)] -pub enum LbaStatus { - /// The block is mapped. - Mapped, - /// The block is deallocated. - Deallocated, - /// The block is anchored. - Anchored, -} - -/// Result of a get LBA status request. -#[derive(Debug, Default, Copy, Clone)] -pub struct DeviceBlockIndexInfo { - /// The size of the first partial block. - pub first_partial_block_size: u32, - /// The index of the first full block. - pub first_full_block_index: u32, - /// The number of blocks. - pub block_count: u32, - /// The size of the last partial block. - pub last_partial_block_size: u32, - /// The number of LBAs per block. - pub lba_per_block: u64, -} - /// Unmap disk sectors that are no longer in use. pub trait Unmap: Send + Sync { /// Unmaps the specified sectors. @@ -505,7 +426,6 @@ trait DynDisk: Send + Sync + Inspect { fn unmap(&self, sector_offset: u64, sector_count: u64, block_level_only: bool) -> IoFuture<'_>; fn optimal_unmap_sectors(&self) -> u32; - fn lba_status(&self) -> Option<&dyn GetLbaStatus>; fn pr(&self) -> Option<&dyn pr::PersistentReservation>; fn eject(&self) -> IoFuture<'_>; @@ -556,10 +476,6 @@ impl DynDisk for T { self.unmap().unwrap().optimal_unmap_sectors() } - fn lba_status(&self) -> Option<&dyn GetLbaStatus> { - self.lba_status() - } - fn pr(&self) -> Option<&dyn pr::PersistentReservation> { self.pr() } diff --git a/vm/devices/storage/disk_blockdevice/src/lib.rs b/vm/devices/storage/disk_blockdevice/src/lib.rs index 5f8189c6ec..4753072161 100644 --- a/vm/devices/storage/disk_blockdevice/src/lib.rs +++ b/vm/devices/storage/disk_blockdevice/src/lib.rs @@ -23,7 +23,6 @@ use disk_backend::resolve::ResolveDiskParameters; use disk_backend::resolve::ResolvedDisk; use disk_backend::DiskError; use disk_backend::DiskIo; -use disk_backend::GetLbaStatus; use disk_backend::Unmap; use fs_err::PathExt; use guestmem::MemoryRead; @@ -527,10 +526,6 @@ impl DiskIo for BlockDevice { (self.optimal_unmap_sectors != 0).then_some(self) } - fn lba_status(&self) -> Option<&dyn GetLbaStatus> { - Some(self) - } - fn pr(&self) -> Option<&dyn PersistentReservation> { if self.supports_pr { Some(self) @@ -710,8 +705,6 @@ impl Unmap for BlockDevice { } } -impl GetLbaStatus for BlockDevice {} - #[async_trait::async_trait] impl PersistentReservation for BlockDevice { fn capabilities(&self) -> ReservationCapabilities { diff --git a/vm/devices/storage/disk_crypt/src/lib.rs b/vm/devices/storage/disk_crypt/src/lib.rs index 0a0d4f974f..1bba64b890 100644 --- a/vm/devices/storage/disk_crypt/src/lib.rs +++ b/vm/devices/storage/disk_crypt/src/lib.rs @@ -94,11 +94,6 @@ impl DiskIo for CryptDisk { self.inner.unmap() } - /// Optionally returns a trait object to issue get LBA status requests. - fn lba_status(&self) -> Option<&dyn disk_backend::GetLbaStatus> { - self.inner.lba_status() - } - /// Optionally returns a trait object to issue persistent reservation /// requests. fn pr(&self) -> Option<&dyn disk_backend::pr::PersistentReservation> { diff --git a/vm/devices/storage/disk_nvme/src/lib.rs b/vm/devices/storage/disk_nvme/src/lib.rs index cf5c2a01a0..24d687d452 100644 --- a/vm/devices/storage/disk_nvme/src/lib.rs +++ b/vm/devices/storage/disk_nvme/src/lib.rs @@ -9,7 +9,6 @@ use async_trait::async_trait; use disk_backend::pr; use disk_backend::DiskError; use disk_backend::DiskIo; -use disk_backend::GetLbaStatus; use disk_backend::MediumErrorDetails; use disk_backend::Unmap; use inspect::Inspect; @@ -70,10 +69,6 @@ impl DiskIo for NvmeDisk { self.namespace.supports_dataset_management().then_some(self) } - fn lba_status(&self) -> Option<&dyn GetLbaStatus> { - Some(self) - } - fn pr(&self) -> Option<&dyn pr::PersistentReservation> { (u8::from(self.namespace.reservation_capabilities()) != 0).then_some(self) } @@ -155,8 +150,6 @@ impl DiskIo for NvmeDisk { } } -impl GetLbaStatus for NvmeDisk {} - impl Unmap for NvmeDisk { async fn unmap( &self, diff --git a/vm/devices/storage/disk_prwrap/src/lib.rs b/vm/devices/storage/disk_prwrap/src/lib.rs index 4990dda4dd..48e1701cc8 100644 --- a/vm/devices/storage/disk_prwrap/src/lib.rs +++ b/vm/devices/storage/disk_prwrap/src/lib.rs @@ -126,10 +126,6 @@ impl DiskIo for DiskWithReservations { // TODO: Implement unmap - fn lba_status(&self) -> Option<&dyn disk_backend::GetLbaStatus> { - self.inner.lba_status() - } - fn pr(&self) -> Option<&dyn pr::PersistentReservation> { Some(self) } diff --git a/vm/devices/storage/disk_striped/src/lib.rs b/vm/devices/storage/disk_striped/src/lib.rs index 9044aa54c6..59e79295fc 100644 --- a/vm/devices/storage/disk_striped/src/lib.rs +++ b/vm/devices/storage/disk_striped/src/lib.rs @@ -12,7 +12,6 @@ use disk_backend::resolve::ResolvedDisk; use disk_backend::Disk; use disk_backend::DiskError; use disk_backend::DiskIo; -use disk_backend::GetLbaStatus; use disk_backend::Unmap; use disk_backend_resources::StripedDiskHandle; use futures::future::join_all; @@ -368,10 +367,6 @@ impl DiskIo for StripedDisk { self.supports_unmap.then_some(self) } - fn lba_status(&self) -> Option<&dyn GetLbaStatus> { - Some(self) - } - fn disk_id(&self) -> Option<[u8; 16]> { None } @@ -572,8 +567,6 @@ impl Unmap for StripedDisk { } } -impl GetLbaStatus for StripedDisk {} - async fn await_all_and_check(futures: T) -> Result<(), E> where T: IntoIterator, diff --git a/vm/devices/storage/scsidisk/src/getlbastatus.rs b/vm/devices/storage/scsidisk/src/getlbastatus.rs index 8ae9e886b5..f9e05282d4 100644 --- a/vm/devices/storage/scsidisk/src/getlbastatus.rs +++ b/vm/devices/storage/scsidisk/src/getlbastatus.rs @@ -1,9 +1,14 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +//! Support for the SCSI "Get LBA Status" command. +//! +//! Currently, this command just returns that all blocks are "mapped". + use super::ScsiError; use super::SimpleScsiDisk; -use disk_backend::LbaStatus; +use disk_backend::Disk; +use disk_backend::DiskError; use guestmem::MemoryWrite; use scsi::AdditionalSenseCode; use scsi_buffers::RequestBuffers; @@ -13,6 +18,72 @@ use zerocopy::AsBytes; use zerocopy::FromBytes; use zerocopy::FromZeroes; +/// Result of a get LBA status request. +#[derive(Debug, Default, Copy, Clone)] +struct DeviceBlockIndexInfo { + /// The size of the first partial block. + first_partial_block_size: u32, + /// The index of the first full block. + first_full_block_index: u32, + /// The number of blocks. + block_count: u32, + /// The size of the last partial block. + #[allow(dead_code)] + last_partial_block_size: u32, + /// The number of LBAs per block. + lba_per_block: u64, +} + +/// The LBA status of a block. +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +enum LbaStatus { + /// The block is mapped. + Mapped, + /// The block is deallocated. + #[allow(dead_code)] + Deallocated, + /// The block is anchored. + #[allow(dead_code)] + Anchored, +} + +/// Returns the block index information for the given file offset. +fn file_offset_to_device_block_index_and_length( + disk: &Disk, + _start_offset: u64, + _get_lba_status_range_length: u64, + _block_size: u64, +) -> DeviceBlockIndexInfo { + let sector_size = disk.sector_size() as u64; + let sector_count = disk.sector_count(); + let disk_size = sector_size * sector_count; + + // Treat fully allocation disk or fixed disk as one large block and just return + // enough descriptors from the LBA requested till the last LBA on disk. + // + // LbaPerBlock is a ULONG and technically with MAXULONG * 512 byte sectors, + // we can get upto 1.99 TB. The LBA descriptor also holds a ULONG + // LogicalBlockCount and can have an issue for larger than 2TB disks. + let lba_per_block = std::cmp::min(sector_count, u32::MAX.into()); + let block_size_large = lba_per_block * sector_size; + let block_count = ((disk_size + block_size_large - 1) / block_size_large) as u32; + DeviceBlockIndexInfo { + first_partial_block_size: 0, + first_full_block_index: 0, + block_count, + last_partial_block_size: 0, + lba_per_block, + } +} + +/// Returns the LBA status for the given block number. +fn get_block_lba_status( + _block_number: u32, + _leaf_node_state_only: bool, +) -> Result { + Ok(LbaStatus::Mapped) +} + impl SimpleScsiDisk { pub(crate) fn handle_get_lba_status( &self, @@ -57,8 +128,7 @@ impl SimpleScsiDisk { let mut lba_count_remaining = total_lba_count_requested; - let lba_status = self.disk.lba_status().unwrap(); - let mut block_index_info = lba_status.file_offset_to_device_block_index_and_length( + let mut block_index_info = file_offset_to_device_block_index_and_length( &self.disk, start_offset, get_lba_status_range_length, @@ -118,11 +188,11 @@ impl SimpleScsiDisk { // Get the LBA status for the very first block. let leaf_node_state_only = request.srb_flags & scsi::SRB_FLAGS_CONSOLIDATEABLE_BLOCKS_ONLY != 0; - let mut provisioning_status = - match lba_status.get_block_lba_status(block_number, leaf_node_state_only) { - Ok(status) => status, - Err(e) => return Err(ScsiError::Disk(e)), - }; + let mut provisioning_status = match get_block_lba_status(block_number, leaf_node_state_only) + { + Ok(status) => status, + Err(e) => return Err(ScsiError::Disk(e)), + }; let mut provisioning_status_in_previous_block; let mut lba_count_in_current_block; @@ -150,11 +220,11 @@ impl SimpleScsiDisk { while block_number < block_number_max { // Get the LBA status for the next block. // Usually it's a full block except when we get to the last block. - provisioning_status = - match lba_status.get_block_lba_status(block_number, leaf_node_state_only) { - Ok(status) => status, - Err(e) => return Err(ScsiError::Disk(e)), - }; + provisioning_status = match get_block_lba_status(block_number, leaf_node_state_only) + { + Ok(status) => status, + Err(e) => return Err(ScsiError::Disk(e)), + }; // This block has a different status from what we are looking for. // Break out of here so we can composite a new LBA_STATUS_DESCRIPTOR. diff --git a/vm/devices/storage/scsidisk/src/lib.rs b/vm/devices/storage/scsidisk/src/lib.rs index d7a4e7a371..ee0fa0fe78 100644 --- a/vm/devices/storage/scsidisk/src/lib.rs +++ b/vm/devices/storage/scsidisk/src/lib.rs @@ -111,7 +111,6 @@ pub struct SimpleScsiDisk { sector_size: u32, sense_data: SenseDataSlot, scsi_parameters: ScsiParameters, - support_get_lba_status: bool, support_pr: bool, last_sector_count: AtomicU64, } @@ -124,6 +123,7 @@ struct ScsiParameters { write_cache_enabled: bool, support_odx: bool, support_unmap: bool, + support_get_lba_status: bool, maximum_transfer_length: usize, identity: DiskIdentity, serial_number: Vec, @@ -158,6 +158,7 @@ impl SimpleScsiDisk { unmap, max_transfer_length, optimal_unmap_sectors, + get_lba_status, } = disk_parameters; fn nonzero_id(id: [u8; 16]) -> Option<[u8; 16]> { @@ -183,6 +184,7 @@ impl SimpleScsiDisk { write_cache_enabled: write_cache.unwrap_or(true), support_odx: odx.unwrap_or(false), support_unmap: unmap.unwrap_or(disk.unmap().is_some()), + support_get_lba_status: get_lba_status, maximum_transfer_length: max_transfer_length.unwrap_or(8 * 1024 * 1024), identity: identity.unwrap_or_else(DiskIdentity::msft), serial_number, @@ -193,7 +195,6 @@ impl SimpleScsiDisk { let physical_extra_shift = scsi_parameters.physical_sector_size.trailing_zeros() as u8 - sector_shift; - let support_get_lba_status = disk.lba_status().is_some(); let support_pr = disk.pr().is_some(); SimpleScsiDisk { @@ -203,7 +204,6 @@ impl SimpleScsiDisk { sector_size, sense_data: Default::default(), scsi_parameters, - support_get_lba_status, support_pr, last_sector_count: AtomicU64::new(sector_count), } @@ -572,7 +572,7 @@ impl SimpleScsiDisk { Ok(tx) } scsi::SERVICE_ACTION_GET_LBA_STATUS => { - if !self.support_get_lba_status { + if !self.scsi_parameters.support_get_lba_status { tracing::debug!("doesn't support get lba status"); Err(ScsiError::IllegalRequest( AdditionalSenseCode::ILLEGAL_COMMAND, @@ -1298,7 +1298,6 @@ impl Inspect for SimpleScsiDisk { self.last_sector_count.load(Ordering::Relaxed), ) .field("scsi_parameters", &self.scsi_parameters) - .field("lba", self.support_get_lba_status) .field("pr", self.support_pr) .field("backend", &self.disk); } diff --git a/vm/devices/storage/scsidisk_resources/src/lib.rs b/vm/devices/storage/scsidisk_resources/src/lib.rs index 0b073f9d82..8ca8789f26 100644 --- a/vm/devices/storage/scsidisk_resources/src/lib.rs +++ b/vm/devices/storage/scsidisk_resources/src/lib.rs @@ -63,6 +63,11 @@ pub struct DiskParameters { pub max_transfer_length: Option, /// The minimum optimal number of sectors to unmap in a request. pub optimal_unmap_sectors: Option, + /// Report LBA status to the guest. + /// + /// Note that this will always report fully mapped LBAs, since the + /// underlying disk implementation has no mechanism to report unmapped LBAs. + pub get_lba_status: bool, } /// The disk identity.