From 60a141b51c27219984f2d4e1712e8e544d9f2d52 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Fri, 7 Mar 2025 22:49:22 +0100 Subject: [PATCH] storage: Move most partition handling into DiskWrapper --- storage/ahcid/src/scheme.rs | 57 ++++----------------- storage/bcm2835-sdhcid/src/scheme.rs | 56 ++++---------------- storage/driver-block/src/lib.rs | 76 +++++++++++++++++++++++++--- storage/ided/src/scheme.rs | 56 ++++---------------- storage/nvmed/src/scheme.rs | 52 ++++--------------- storage/virtio-blkd/src/scheme.rs | 36 +++++-------- 6 files changed, 121 insertions(+), 212 deletions(-) diff --git a/storage/ahcid/src/scheme.rs b/storage/ahcid/src/scheme.rs index 63d432e9a8..74dc53067e 100644 --- a/storage/ahcid/src/scheme.rs +++ b/storage/ahcid/src/scheme.rs @@ -7,8 +7,8 @@ use driver_block::{Disk, DiskWrapper}; use redox_scheme::{CallerCtx, OpenResult, SchemeBlock}; use syscall::schemev2::NewFdFlags; use syscall::{ - Error, Result, Stat, EACCES, EBADF, EISDIR, ENOENT, ENOLCK, EOVERFLOW, MODE_DIR, MODE_FILE, - O_DIRECTORY, O_STAT, + Error, Result, Stat, EACCES, EBADF, EISDIR, ENOENT, ENOLCK, MODE_DIR, MODE_FILE, O_DIRECTORY, + O_STAT, }; use crate::ahci::hba::HbaMem; @@ -263,32 +263,13 @@ impl SchemeBlock for DiskScheme { } Handle::Disk(number) => { let disk = self.disks.get_mut(number).ok_or(Error::new(EBADF))?; - let blk_len = disk.block_size(); - disk.read(offset / u64::from(blk_len), buf) + let block = offset / u64::from(disk.block_size()); + disk.read(None, block, buf) } Handle::Partition(disk_num, part_num) => { let disk = self.disks.get_mut(disk_num).ok_or(Error::new(EBADF))?; - let blksize = disk.block_size(); - - // validate that we're actually reading within the bounds of the partition - let rel_block = offset / u64::from(blksize); - - let abs_block = { - let pt = disk.pt.as_ref().ok_or(Error::new(EBADF))?; - let partition = pt - .partitions - .get(part_num as usize) - .ok_or(Error::new(EBADF))?; - - let abs_block = partition.start_lba + rel_block; - // TODO: This shouldn't return EOVERFLOW? - if rel_block >= partition.size { - return Err(Error::new(EOVERFLOW)); - } - abs_block - }; - - disk.read(abs_block, buf) + let block = offset / u64::from(disk.block_size()); + disk.read(Some(part_num as usize), block, buf) } } } @@ -304,31 +285,13 @@ impl SchemeBlock for DiskScheme { Handle::List(_) => Err(Error::new(EBADF)), Handle::Disk(number) => { let disk = self.disks.get_mut(number).ok_or(Error::new(EBADF))?; - let blk_len = disk.block_size(); - disk.write(offset / u64::from(blk_len), buf) + let block = offset / u64::from(disk.block_size()); + disk.write(None, block, buf) } Handle::Partition(disk_num, part_num) => { let disk = self.disks.get_mut(disk_num).ok_or(Error::new(EBADF))?; - let blksize = disk.block_size(); - - // validate that we're actually reading within the bounds of the partition - let rel_block = offset / u64::from(blksize); - - let abs_block = { - let pt = disk.pt.as_ref().ok_or(Error::new(EBADF))?; - let partition = pt - .partitions - .get(part_num as usize) - .ok_or(Error::new(EBADF))?; - - let abs_block = partition.start_lba + rel_block; - if rel_block >= partition.size { - return Err(Error::new(EOVERFLOW)); - } - abs_block - }; - - disk.write(abs_block, buf) + let block = offset / u64::from(disk.block_size()); + disk.write(Some(part_num as usize), block, buf) } } } diff --git a/storage/bcm2835-sdhcid/src/scheme.rs b/storage/bcm2835-sdhcid/src/scheme.rs index ebe0612346..dfd7264786 100644 --- a/storage/bcm2835-sdhcid/src/scheme.rs +++ b/storage/bcm2835-sdhcid/src/scheme.rs @@ -5,8 +5,8 @@ use std::str; use driver_block::{Disk, DiskWrapper}; use syscall::schemev2::NewFdFlags; use syscall::{ - Error, Result, Stat, EACCES, EBADF, EISDIR, ENOENT, ENOLCK, EOVERFLOW, MODE_DIR, MODE_FILE, - O_DIRECTORY, O_STAT, + Error, Result, Stat, EACCES, EBADF, EISDIR, ENOENT, ENOLCK, MODE_DIR, MODE_FILE, O_DIRECTORY, + O_STAT, }; use redox_scheme::{CallerCtx, OpenResult, SchemeBlock}; @@ -251,31 +251,13 @@ impl SchemeBlock for DiskScheme { } Handle::Disk(number) => { let disk = self.disks.get_mut(number).ok_or(Error::new(EBADF))?; - let blk_len = disk.block_size(); - disk.read(offset / u64::from(blk_len), buf) + let block = offset / u64::from(disk.block_size()); + disk.read(None, block, buf) } Handle::Partition(disk_num, part_num) => { let disk = self.disks.get_mut(disk_num).ok_or(Error::new(EBADF))?; - let blksize = disk.block_size(); - - // validate that we're actually reading within the bounds of the partition - let rel_block = offset / u64::from(blksize); - - let abs_block = { - let pt = disk.pt.as_ref().ok_or(Error::new(EBADF))?; - let partition = pt - .partitions - .get(part_num as usize) - .ok_or(Error::new(EBADF))?; - - let abs_block = partition.start_lba + rel_block; - if rel_block >= partition.size { - return Err(Error::new(EOVERFLOW)); - } - abs_block - }; - - disk.read(abs_block, buf) + let block = offset / u64::from(disk.block_size()); + disk.read(Some(part_num as usize), block, buf) } } } @@ -285,31 +267,13 @@ impl SchemeBlock for DiskScheme { Handle::List(_) => Err(Error::new(EBADF)), Handle::Disk(number) => { let disk = self.disks.get_mut(number).ok_or(Error::new(EBADF))?; - let blk_len = disk.block_size(); - disk.write(offset / u64::from(blk_len), buf) + let block = offset / u64::from(disk.block_size()); + disk.write(None, block, buf) } Handle::Partition(disk_num, part_num) => { let disk = self.disks.get_mut(disk_num).ok_or(Error::new(EBADF))?; - let blksize = disk.block_size(); - - // validate that we're actually reading within the bounds of the partition - let rel_block = offset / u64::from(blksize as u64); - - let abs_block = { - let pt = disk.pt.as_ref().ok_or(Error::new(EBADF))?; - let partition = pt - .partitions - .get(part_num as usize) - .ok_or(Error::new(EBADF))?; - - let abs_block = partition.start_lba + rel_block; - if rel_block >= partition.size { - return Err(Error::new(EOVERFLOW)); - } - abs_block - }; - - disk.write(abs_block, buf) + let block = offset / u64::from(disk.block_size()); + disk.write(Some(part_num as usize), block, buf) } } } diff --git a/storage/driver-block/src/lib.rs b/storage/driver-block/src/lib.rs index 1eb165d185..a2910c7b11 100644 --- a/storage/driver-block/src/lib.rs +++ b/storage/driver-block/src/lib.rs @@ -3,6 +3,7 @@ use std::io::Error; use std::io::{self, Read, Seek, SeekFrom}; use partitionlib::{LogicalBlockSize, PartitionTable}; +use syscall::{EBADF, EOVERFLOW}; /// Split the read operation into a series of block reads. /// `read_fn` will be called with a block number to be read, and a buffer to be filled. @@ -157,17 +158,76 @@ impl DiskWrapper { disk, } } -} -impl std::ops::Deref for DiskWrapper { - type Target = T; - - fn deref(&self) -> &Self::Target { + pub fn disk(&self) -> &T { &self.disk } -} -impl std::ops::DerefMut for DiskWrapper { - fn deref_mut(&mut self) -> &mut T { + + pub fn disk_mut(&mut self) -> &mut T { &mut self.disk } + + pub fn block_size(&self) -> u32 { + self.disk.block_size() + } + + pub fn size(&self) -> u64 { + self.disk.size() + } + + pub fn read( + &mut self, + part_num: Option, + block: u64, + buf: &mut [u8], + ) -> syscall::Result> { + if let Some(part_num) = part_num { + let part = self + .pt + .as_ref() + .ok_or(syscall::Error::new(EBADF))? + .partitions + .get(part_num) + .ok_or(syscall::Error::new(EBADF))?; + + let block_size = u64::from(self.block_size()); + if block >= part.size / block_size { + return Err(syscall::Error::new(EOVERFLOW)); + } + + let abs_block = part.start_lba + block; + + self.disk.read(abs_block, buf) + } else { + self.disk.read(block, buf) + } + } + + pub fn write( + &mut self, + part_num: Option, + block: u64, + buf: &[u8], + ) -> syscall::Result> { + if let Some(part_num) = part_num { + let part = self + .pt + .as_ref() + .ok_or(syscall::Error::new(EBADF))? + .partitions + .get(part_num) + .ok_or(syscall::Error::new(EBADF))?; + + let block_size = u64::from(self.block_size()); + if block >= part.size / block_size { + return Err(syscall::Error::new(EOVERFLOW)); + } + + let abs_block = part.start_lba + block; + + self.disk.write(abs_block, buf) + } else { + self.disk.write(block, buf) + } + } } diff --git a/storage/ided/src/scheme.rs b/storage/ided/src/scheme.rs index 167eb78a80..f41a227579 100644 --- a/storage/ided/src/scheme.rs +++ b/storage/ided/src/scheme.rs @@ -6,8 +6,8 @@ use std::sync::{Arc, Mutex}; use driver_block::{Disk, DiskWrapper}; use syscall::schemev2::NewFdFlags; use syscall::{ - Error, Result, Stat, EACCES, EBADF, EISDIR, ENOENT, ENOLCK, EOVERFLOW, MODE_DIR, MODE_FILE, - O_DIRECTORY, O_STAT, + Error, Result, Stat, EACCES, EBADF, EISDIR, ENOENT, ENOLCK, MODE_DIR, MODE_FILE, O_DIRECTORY, + O_STAT, }; use crate::ide::Channel; @@ -265,31 +265,13 @@ impl SchemeBlock for DiskScheme { } Handle::Disk(number) => { let disk = self.disks.get_mut(number).ok_or(Error::new(EBADF))?; - let blk_len = disk.block_size(); - disk.read(offset / u64::from(blk_len), buf) + let block = offset / u64::from(disk.block_size()); + disk.read(None, block, buf) } Handle::Partition(disk_num, part_num) => { let disk = self.disks.get_mut(disk_num).ok_or(Error::new(EBADF))?; - let blksize = disk.block_size(); - - // validate that we're actually reading within the bounds of the partition - let rel_block = offset / u64::from(blksize); - - let abs_block = { - let pt = disk.pt.as_ref().ok_or(Error::new(EBADF))?; - let partition = pt - .partitions - .get(part_num as usize) - .ok_or(Error::new(EBADF))?; - - let abs_block = partition.start_lba + rel_block; - if rel_block >= partition.size { - return Err(Error::new(EOVERFLOW)); - } - abs_block - }; - - disk.read(abs_block, buf) + let block = offset / u64::from(disk.block_size()); + disk.read(Some(part_num as usize), block, buf) } } } @@ -299,31 +281,13 @@ impl SchemeBlock for DiskScheme { Handle::List(_) => Err(Error::new(EBADF)), Handle::Disk(number) => { let disk = self.disks.get_mut(number).ok_or(Error::new(EBADF))?; - let blk_len = disk.block_size(); - disk.write(offset / u64::from(blk_len), buf) + let block = offset / u64::from(disk.block_size()); + disk.write(None, block, buf) } Handle::Partition(disk_num, part_num) => { let disk = self.disks.get_mut(disk_num).ok_or(Error::new(EBADF))?; - let blksize = disk.block_size(); - - // validate that we're actually reading within the bounds of the partition - let rel_block = offset / u64::from(blksize as u64); - - let abs_block = { - let pt = disk.pt.as_ref().ok_or(Error::new(EBADF))?; - let partition = pt - .partitions - .get(part_num as usize) - .ok_or(Error::new(EBADF))?; - - let abs_block = partition.start_lba + rel_block; - if rel_block >= partition.size { - return Err(Error::new(EOVERFLOW)); - } - abs_block - }; - - disk.write(abs_block, buf) + let block = offset / u64::from(disk.block_size()); + disk.write(Some(part_num as usize), block, buf) } } } diff --git a/storage/nvmed/src/scheme.rs b/storage/nvmed/src/scheme.rs index 48018b33e5..bd864441dd 100644 --- a/storage/nvmed/src/scheme.rs +++ b/storage/nvmed/src/scheme.rs @@ -8,8 +8,8 @@ use driver_block::{Disk, DiskWrapper}; use redox_scheme::{CallerCtx, OpenResult, SchemeBlock}; use syscall::schemev2::NewFdFlags; use syscall::{ - Error, Result, Stat, EACCES, EBADF, EISDIR, ENOENT, ENOLCK, EOVERFLOW, MODE_DIR, MODE_FILE, - O_DIRECTORY, O_STAT, + Error, Result, Stat, EACCES, EBADF, EISDIR, ENOENT, ENOLCK, MODE_DIR, MODE_FILE, O_DIRECTORY, + O_STAT, }; use crate::nvme::{Nvme, NvmeNamespace}; @@ -183,7 +183,7 @@ impl SchemeBlock for DiskScheme { Handle::Disk(number) => { let disk = self.disks.get_mut(&number).ok_or(Error::new(EBADF))?; stat.st_mode = MODE_FILE; - stat.st_blocks = disk.1.blocks; + stat.st_blocks = disk.disk().1.blocks; stat.st_blksize = disk.block_size(); stat.st_size = disk.size(); Ok(Some(0)) @@ -270,28 +270,13 @@ impl SchemeBlock for DiskScheme { } Handle::Disk(number) => { let disk = self.disks.get_mut(&number).ok_or(Error::new(EBADF))?; - let block_size = u64::from(disk.block_size()); - disk.read(offset / block_size, buf) + let block = offset / u64::from(disk.block_size()); + disk.read(None, block, buf) } Handle::Partition(disk_num, part_num) => { let disk = self.disks.get_mut(&disk_num).ok_or(Error::new(EBADF))?; - let part = disk - .pt - .as_ref() - .ok_or(Error::new(EBADF))? - .partitions - .get(part_num as usize) - .ok_or(Error::new(EBADF))?; - - let block_size = u64::from(disk.block_size()); - let rel_block = offset / block_size; - if rel_block >= part.size { - return Err(Error::new(EOVERFLOW)); - } - - let abs_block = part.start_lba + rel_block; - - disk.read(abs_block, buf) + let block = offset / u64::from(disk.block_size()); + disk.read(Some(part_num as usize), block, buf) } } } @@ -307,28 +292,13 @@ impl SchemeBlock for DiskScheme { Handle::List(_) => Err(Error::new(EBADF)), Handle::Disk(number) => { let disk = self.disks.get_mut(&number).ok_or(Error::new(EBADF))?; - let block_size = u64::from(disk.block_size()); - disk.write(offset / block_size, buf) + let block = offset / u64::from(disk.block_size()); + disk.write(None, block, buf) } Handle::Partition(disk_num, part_num) => { let disk = self.disks.get_mut(&disk_num).ok_or(Error::new(EBADF))?; - let part = disk - .pt - .as_ref() - .ok_or(Error::new(EBADF))? - .partitions - .get(part_num as usize) - .ok_or(Error::new(EBADF))?; - - let block_size = u64::from(disk.block_size()); - let rel_block = offset / block_size; - if rel_block >= part.size { - return Err(Error::new(EOVERFLOW)); - } - - let abs_block = part.start_lba + rel_block; - - disk.write(abs_block, buf) + let block = offset / u64::from(disk.block_size()); + disk.write(Some(part_num as usize), block, buf) } } } diff --git a/storage/virtio-blkd/src/scheme.rs b/storage/virtio-blkd/src/scheme.rs index f47abc33c6..974573da51 100644 --- a/storage/virtio-blkd/src/scheme.rs +++ b/storage/virtio-blkd/src/scheme.rs @@ -4,7 +4,7 @@ use std::fmt::Write; use std::sync::Arc; use common::dma::Dma; -use driver_block::{Disk, DiskWrapper}; +use driver_block::DiskWrapper; use redox_scheme::CallerCtx; use redox_scheme::OpenResult; @@ -239,28 +239,13 @@ impl<'a> SchemeBlock for DiskScheme<'a> { buf[..count].copy_from_slice(&src[..count]); Ok(Some(count)) } - - Handle::Partition { number } => { - let part_table = self.disk.pt.as_ref().unwrap(); - let part = part_table - .partitions - .get(number as usize) - .ok_or(Error::new(EBADF))?; - - // Get the offset in sectors. - let rel_block = offset / BLK_SIZE; - // if rel_block >= part.size { - // return Err(Error::new(EOVERFLOW)); - // } - - let abs_block = part.start_lba + rel_block; - - self.disk.read(abs_block, buf) - } - Handle::Disk => { let block = offset / u64::from(self.disk.block_size()); - self.disk.read(block, buf) + self.disk.read(None, block, buf) + } + Handle::Partition { number } => { + let block = offset / u64::from(self.disk.block_size()); + self.disk.read(Some(number as usize), block, buf) } } } @@ -273,12 +258,15 @@ impl<'a> SchemeBlock for DiskScheme<'a> { _fcntl_flags: u32, ) -> syscall::Result> { match *self.handles.get_mut(&id).ok_or(Error::new(EBADF))? { + Handle::List { .. } => Err(Error::new(EBADF)), Handle::Disk => { let block = offset / u64::from(self.disk.block_size()); - self.disk.write(block, buf) + self.disk.write(None, block, buf) + } + Handle::Partition { number } => { + let block = offset / u64::from(self.disk.block_size()); + self.disk.write(Some(number as usize), block, buf) } - - _ => todo!(), } }