diff --git a/block/src/fixed_vhd_disk.rs b/block/src/fixed_vhd_disk.rs index 8a27cdf96..d8c0332be 100644 --- a/block/src/fixed_vhd_disk.rs +++ b/block/src/fixed_vhd_disk.rs @@ -1,5 +1,7 @@ // Copyright 2026 The Cloud Hypervisor Authors. All rights reserved. // +// Copyright (c) Meta Platforms, Inc. and affiliates. +// // SPDX-License-Identifier: Apache-2.0 use std::fs::File; @@ -120,11 +122,15 @@ impl disk_file::AsyncDiskFile for FixedVhdDisk { mod unit_tests { use std::fs::File; use std::io::{Seek, SeekFrom, Write}; + #[cfg(feature = "io_uring")] + use std::os::fd::AsRawFd; use vmm_sys_util::tempfile::TempFile; use super::*; use crate::async_io::AsyncIo; + #[cfg(feature = "io_uring")] + use crate::async_io::{AsyncIoOperation, OwnedIoBuffer}; use crate::disk_file::{AsyncDiskFile, DiskSize, PhysicalSize, Resizable}; /// Minimal fixed VHD footer (disk type = 2, current_size = 0x11223344). @@ -188,6 +194,20 @@ mod unit_tests { assert_async_io(&disk, true); } + #[cfg(feature = "io_uring")] + #[test] + fn io_uring_batch_rejects_request_past_logical_size() { + let file = TempFile::new().unwrap().into_file(); + file.set_len(0x2000).unwrap(); + let mut async_io = FixedVhdAsync::new(file.as_raw_fd(), 8, 0x1000).unwrap(); + let op = AsyncIoOperation::read_to_vec(0x800, OwnedIoBuffer::from_vec(vec![0; 0x900]), 1); + + assert!(matches!( + async_io.submit_batch_operations(vec![op]), + Err(crate::async_io::AsyncIoError::ReadVectored(_)) + )); + } + #[test] fn try_clone_preserves_sync_dispatch() { let file = make_vhd_file(); diff --git a/block/src/raw_async_io_tests.rs b/block/src/raw_async_io_tests.rs index 560e41e33..393adf4b4 100644 --- a/block/src/raw_async_io_tests.rs +++ b/block/src/raw_async_io_tests.rs @@ -1,5 +1,7 @@ // Copyright 2026 The Cloud Hypervisor Authors. All rights reserved. // +// Copyright (c) Meta Platforms, Inc. and affiliates. +// // SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause //! Shared test helpers for [`AsyncIo`] backends. @@ -13,6 +15,11 @@ use std::io::{Read, Seek, SeekFrom, Write}; use crate::async_io::{AsyncIo, AsyncIoError}; +fn next_completion(async_io: &mut dyn AsyncIo) -> (u64, i32) { + let completion = async_io.next_completion().expect("No completion"); + (completion.user_data, completion.result) +} + /// Tests punching a hole in the middle of a 4 MB file and verifying data /// integrity around the hole. pub fn test_punch_hole(async_io: &mut dyn AsyncIo, file: &mut File) { @@ -27,7 +34,7 @@ pub fn test_punch_hole(async_io: &mut dyn AsyncIo, file: &mut File) { async_io.punch_hole(offset, length, 1).unwrap(); // Check completion - let (user_data, result) = async_io.next_completed_request().unwrap(); + let (user_data, result) = next_completion(async_io); assert_eq!(user_data, 1); assert_eq!(result, 0); @@ -84,7 +91,7 @@ pub fn test_write_zeroes(async_io: &mut dyn AsyncIo, file: &mut File) { write_zeroes_result.unwrap(); // Check completion - let (user_data, result) = async_io.next_completed_request().unwrap(); + let (user_data, result) = next_completion(async_io); assert_eq!(user_data, 2); assert_eq!(result, 0); @@ -134,15 +141,15 @@ pub fn test_punch_hole_multiple_operations(async_io: &mut dyn AsyncIo, file: &mu .unwrap(); // Check all completions - let (user_data, result) = async_io.next_completed_request().unwrap(); + let (user_data, result) = next_completion(async_io); assert_eq!(user_data, 10); assert_eq!(result, 0); - let (user_data, result) = async_io.next_completed_request().unwrap(); + let (user_data, result) = next_completion(async_io); assert_eq!(user_data, 11); assert_eq!(result, 0); - let (user_data, result) = async_io.next_completed_request().unwrap(); + let (user_data, result) = next_completion(async_io); assert_eq!(user_data, 12); assert_eq!(result, 0); diff --git a/block/src/request.rs b/block/src/request.rs index d8dce1513..af7d08c83 100644 --- a/block/src/request.rs +++ b/block/src/request.rs @@ -29,8 +29,9 @@ use vm_memory::{ }; use vm_virtio::{AccessPlatform, Translatable as _}; -use crate::aligned_operation::AlignedOperation; -use crate::async_io::{AsyncIo, AsyncIoOperation, GuestMemoryTarget, OwnedIoBuffer}; +use crate::async_io::{ + AsyncIo, AsyncIoCompletion, AsyncIoOperation, GuestMemoryTarget, OwnedIoBuffer, +}; use crate::{Error, ExecuteError, request_type, sector}; const SECTOR_SHIFT: u8 = 9; @@ -70,7 +71,7 @@ pub struct ExecuteAsync { // `true` if the execution will complete asynchronously pub async_complete: bool, // request need to be batched for submission if any - pub batch_request: Option, + pub batch_request: Option, } #[derive(Debug)] @@ -80,7 +81,6 @@ pub struct Request { data_descriptors: SmallVec<[(GuestAddress, u32); DEFAULT_DESCRIPTOR_VEC_SIZE]>, status_addr: GuestAddress, pub writeback: bool, - aligned_operations: SmallVec<[AlignedOperation; DEFAULT_DESCRIPTOR_VEC_SIZE]>, start: Instant, } @@ -112,7 +112,6 @@ impl Request { data_descriptors: SmallVec::with_capacity(DEFAULT_DESCRIPTOR_VEC_SIZE), status_addr: GuestAddress(0), writeback: true, - aligned_operations: SmallVec::with_capacity(DEFAULT_DESCRIPTOR_VEC_SIZE), start: Instant::now(), }; @@ -237,9 +236,9 @@ impl Request { Ok(len) } - pub fn execute_async( + pub fn execute_async( &mut self, - mem: &vm_memory::GuestMemoryMmap, + mem: Arc>, disk_nsectors: u64, disk_image: &mut dyn AsyncIo, serial: &[u8], @@ -253,56 +252,6 @@ impl Request { self.check_data_bounds(disk_nsectors)?; - let mut iovecs: SmallVec<[libc::iovec; DEFAULT_DESCRIPTOR_VEC_SIZE]> = - SmallVec::with_capacity(self.data_descriptors.len()); - for &(data_addr, data_len) in &self.data_descriptors { - let _: u32 = data_len; // compiler-checked documentation - const _: () = assert!( - core::mem::size_of::() <= core::mem::size_of::(), - "unsupported platform" - ); - if data_len == 0 { - continue; - } - let data_len = data_len as usize; - - let origin_ptr = mem - .get_slice(data_addr, data_len) - .map_err(ExecuteError::GetHostAddress)?; - assert!(origin_ptr.len() >= data_len); - let origin_ptr = origin_ptr.ptr_guard_mut(); - - // O_DIRECT requires buffer addresses to be aligned to the - // backend device's logical block size. In case it's not properly - // aligned, an intermediate buffer is created with the correct - // alignment, and a copy from/to the origin buffer is performed, - // depending on the type of operation. - let iov_base = if (origin_ptr.as_ptr() as u64).is_multiple_of(alignment) { - origin_ptr.as_ptr().cast() - } else { - let mut aligned_op = AlignedOperation::new(data_addr, data_len, alignment as usize) - .map_err(ExecuteError::TemporaryBufferAllocation)?; - - // We need to perform the copy beforehand in case we're writing - // data out. - if request_type == RequestType::Out { - mem.read_slice(aligned_op.as_bytes_mut(), data_addr) - .map_err(ExecuteError::Read)?; - } - - let aligned_ptr = aligned_op.as_mut_ptr(); - self.aligned_operations.push(aligned_op); - - aligned_ptr.cast() - }; - - let iovec = libc::iovec { - iov_base, - iov_len: data_len as libc::size_t, - }; - iovecs.push(iovec); - } - let mut ret = ExecuteAsync { async_complete: true, batch_request: None, @@ -310,37 +259,52 @@ impl Request { // Queue operations expected to be submitted. match request_type { RequestType::In => { - for (data_addr, data_len) in &self.data_descriptors { - mem.get_slice(*data_addr, *data_len as usize) - .map_err(ExecuteError::GetHostAddress)? - .bitmap() - .mark_dirty(0, *data_len as usize); - } + self.mark_read_dirty(&mem)?; + let op = self.build_data_operation(mem, offset, alignment, user_data)?; if disk_image.batch_requests_enabled() { - ret.batch_request = Some(BatchRequest { - offset, - iovecs, - user_data, - request_type, - }); + ret.batch_request = Some(op); } else { - disk_image - .read_vectored(offset, &iovecs, user_data) - .map_err(ExecuteError::AsyncRead)?; + match op { + AsyncIoOperation::ReadToMemory { + offset, + target, + user_data, + } => disk_image + .read_to_memory(offset, target, user_data) + .map_err(ExecuteError::AsyncRead)?, + AsyncIoOperation::ReadToVec { + offset, + buffer, + user_data, + } => disk_image + .read_to_vec(offset, buffer, user_data) + .map_err(ExecuteError::AsyncRead)?, + _ => unreachable!("unexpected read operation"), + } } } RequestType::Out => { + let op = self.build_data_operation(mem, offset, alignment, user_data)?; if disk_image.batch_requests_enabled() { - ret.batch_request = Some(BatchRequest { - offset, - iovecs, - user_data, - request_type, - }); + ret.batch_request = Some(op); } else { - disk_image - .write_vectored(offset, &iovecs, user_data) - .map_err(ExecuteError::AsyncWrite)?; + match op { + AsyncIoOperation::WriteFromMemory { + offset, + target, + user_data, + } => disk_image + .write_from_memory(offset, target, user_data) + .map_err(ExecuteError::AsyncWrite)?, + AsyncIoOperation::WriteFromVec { + offset, + buffer, + user_data, + } => disk_image + .write_from_vec(offset, buffer, user_data) + .map_err(ExecuteError::AsyncWrite)?, + _ => unreachable!("unexpected write operation"), + } } } RequestType::Flush => { @@ -506,7 +470,6 @@ impl Request { } // Builds a read or write operation for IO to or from `mem`. - #[allow(dead_code)] fn build_data_operation( &self, mem: Arc>, @@ -540,7 +503,6 @@ impl Request { } // Checks whether `self.data_descriptors` are aligned to `alignment`. - #[allow(dead_code)] fn guest_memory_is_aligned( &self, mem: &vm_memory::GuestMemoryMmap, @@ -575,7 +537,6 @@ impl Request { } // Returns the sum of the lengths of `self.data_descriptors`. - #[allow(dead_code)] fn data_len(&self) -> usize { self.data_descriptors .iter() @@ -584,7 +545,6 @@ impl Request { } // Marks guest-memory read destinations dirty before submitting async IO. - #[allow(dead_code)] fn mark_read_dirty( &self, mem: &vm_memory::GuestMemoryMmap, @@ -599,7 +559,6 @@ impl Request { } // Copies guest descriptor contents into a contiguous host buffer. - #[allow(dead_code)] fn copy_guest_to_buffer( &self, mem: &vm_memory::GuestMemoryMmap, @@ -616,7 +575,6 @@ impl Request { } // Copies a host completion buffer back into guest descriptors. - #[allow(dead_code)] fn copy_buffer_to_guest( &self, mem: &vm_memory::GuestMemoryMmap, @@ -638,14 +596,14 @@ impl Request { pub fn complete_async( &mut self, mem: &vm_memory::GuestMemoryMmap, + completion: &mut AsyncIoCompletion, ) -> Result<(), Error> { - for aligned_op in self.aligned_operations.drain(..) { - // We need to perform the copy after the data has been read inside - // the aligned buffer in case we're reading data in. - if self.request_type == RequestType::In { - mem.write_slice(aligned_op.as_bytes(), aligned_op.data_addr()) - .map_err(Error::GuestMemory)?; - } + if self.request_type == RequestType::In + && completion.result > 0 + && let Some(buffer) = completion.buffer.take() + { + let len = (completion.result as usize).min(buffer.as_slice().len()); + self.copy_buffer_to_guest(mem, &buffer.as_slice()[..len])?; } Ok(()) diff --git a/virtio-devices/src/block.rs b/virtio-devices/src/block.rs index f40f1a49a..6cd939ab8 100644 --- a/virtio-devices/src/block.rs +++ b/virtio-devices/src/block.rs @@ -6,6 +6,8 @@ // // Copyright © 2020 Intel Corporation // +// Copyright (c) Meta Platforms, Inc. and affiliates. +// // SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause use std::cmp::max; @@ -303,7 +305,7 @@ impl BlockEpollHandler { request.writeback = self.writeback.load(Ordering::Acquire); let result = request.execute_async( - desc_chain.memory(), + self.mem.memory().into_inner(), self.disk_nsectors.load(Ordering::SeqCst), self.disk_image.as_mut(), &self.serial, @@ -317,15 +319,7 @@ impl BlockEpollHandler { }) = result { if let Some(batch_request) = batch_request { - match batch_request.request_type { - RequestType::In | RequestType::Out => batch_requests.push(batch_request), - _ => { - unreachable!( - "Unexpected batch request type: {:?}", - request.request_type() - ) - } - } + batch_requests.push(batch_request); batch_inflight_requests.push((desc_chain.head_index(), request)); } else { self.inflight_requests @@ -363,24 +357,26 @@ impl BlockEpollHandler { } } - match self.disk_image.submit_batch_requests(&batch_requests) { - Ok(()) => { - self.inflight_requests.extend(batch_inflight_requests); - } - Err(e) => { - // If batch submission fails, report VIRTIO_BLK_S_IOERR for all requests. - for (user_data, request) in batch_inflight_requests { - warn!("Request failed with batch submission: {request:x?} {e:?}"); - let desc_index = user_data; - let mem = self.mem.memory(); - mem.write_obj(VIRTIO_BLK_S_IOERR as u8, request.status_addr()) - .map_err(Error::RequestStatus)?; - queue - .add_used(mem.deref(), desc_index, 1) - .map_err(Error::QueueAddUsed)?; - queue - .enable_notification(mem.deref()) - .map_err(Error::QueueEnableNotification)?; + if !batch_requests.is_empty() { + match self.disk_image.submit_batch_operations(batch_requests) { + Ok(()) => { + self.inflight_requests.extend(batch_inflight_requests); + } + Err(e) => { + // If batch submission fails, report VIRTIO_BLK_S_IOERR for all requests. + for (user_data, request) in batch_inflight_requests { + warn!("Request failed with batch submission: {request:x?} {e:?}"); + let desc_index = user_data; + let mem = self.mem.memory(); + mem.write_obj(VIRTIO_BLK_S_IOERR as u8, request.status_addr()) + .map_err(Error::RequestStatus)?; + queue + .add_used(mem.deref(), desc_index, 1) + .map_err(Error::QueueAddUsed)?; + queue + .enable_notification(mem.deref()) + .map_err(Error::QueueEnableNotification)?; + } } } } @@ -451,13 +447,14 @@ impl BlockEpollHandler { let mut read_ops = Wrapping(0); let mut write_ops = Wrapping(0); - while let Some((user_data, result)) = self.disk_image.next_completed_request() { - let desc_index = user_data as u16; + while let Some(mut completion) = self.disk_image.next_completion() { + let result = completion.result; + let desc_index = completion.user_data as u16; let mut request = self.find_inflight_request(desc_index)?; request - .complete_async(&mem) + .complete_async(&mem, &mut completion) .map_err(Error::RequestCompleting)?; let latency = request.start().elapsed().as_micros() as u64;