From dfe23b06834700847891c69f4831fda3fb8a9a03 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Fri, 7 Mar 2025 21:35:49 +0100 Subject: [PATCH] storage/driver-block: Make Disk::block_length infallible It should be read when creating the struct implementing Disk and every disk needs to have some block length. --- storage/ahcid/src/ahci/disk_ata.rs | 4 +-- storage/ahcid/src/ahci/disk_atapi.rs | 44 ++++++++++++---------------- storage/ahcid/src/scheme.rs | 16 +++++----- storage/bcm2835-sdhcid/src/scheme.rs | 16 +++++----- storage/bcm2835-sdhcid/src/sd/mod.rs | 4 +-- storage/driver-block/src/lib.rs | 11 +++---- storage/ided/src/ide.rs | 4 +-- storage/ided/src/scheme.rs | 16 +++++----- storage/nvmed/src/scheme.rs | 4 +-- storage/virtio-blkd/src/scheme.rs | 4 +-- 10 files changed, 57 insertions(+), 66 deletions(-) diff --git a/storage/ahcid/src/ahci/disk_ata.rs b/storage/ahcid/src/ahci/disk_ata.rs index d5173e2f9e..ee3b32a8b6 100644 --- a/storage/ahcid/src/ahci/disk_ata.rs +++ b/storage/ahcid/src/ahci/disk_ata.rs @@ -178,7 +178,7 @@ impl Disk for DiskATA { } } - fn block_length(&mut self) -> Result { - Ok(512) + fn block_length(&mut self) -> u32 { + 512 } } diff --git a/storage/ahcid/src/ahci/disk_atapi.rs b/storage/ahcid/src/ahci/disk_atapi.rs index 33b35f92b1..ce3ce0b962 100644 --- a/storage/ahcid/src/ahci/disk_atapi.rs +++ b/storage/ahcid/src/ahci/disk_atapi.rs @@ -25,6 +25,8 @@ pub struct DiskATAPI { // Just using the same buffer size as DiskATA // Although the sector size is different (and varies) buf: Dma<[u8; 256 * 512]>, + blk_count: u32, + blk_size: u32, } impl DiskATAPI { @@ -38,12 +40,20 @@ impl DiskATAPI { .unwrap_or_else(|_| unreachable!()); let mut fb = unsafe { Dma::zeroed()?.assume_init() }; - let buf = unsafe { Dma::zeroed()?.assume_init() }; + let mut buf = unsafe { Dma::zeroed()?.assume_init() }; port.init(&mut clb, &mut ctbas, &mut fb); let size = unsafe { port.identify_packet(&mut clb, &mut ctbas).unwrap_or(0) }; + let mut cmd = [0; 16]; + cmd[0] = SCSI_READ_CAPACITY; + port.atapi_dma(&cmd, 8, &mut clb, &mut ctbas, &mut buf)?; + + // Instead of a count, contains number of last LBA, so add 1 + let blk_count = BigEndian::read_u32(&buf[0..4]) + 1; + let blk_size = BigEndian::read_u32(&buf[4..8]); + Ok(DiskATAPI { id, port, @@ -52,37 +62,25 @@ impl DiskATAPI { ctbas, _fb: fb, buf, + blk_count, + blk_size, }) } - - fn read_capacity(&mut self) -> Result<(u32, u32)> { - // TODO: only query when needed (disk changed) - - let mut cmd = [0; 16]; - cmd[0] = SCSI_READ_CAPACITY; - self.port - .atapi_dma(&cmd, 8, &mut self.clb, &mut self.ctbas, &mut self.buf)?; - - // Instead of a count, contains number of last LBA, so add 1 - let blk_count = BigEndian::read_u32(&self.buf[0..4]) + 1; - let blk_size = BigEndian::read_u32(&self.buf[4..8]); - - Ok((blk_count, blk_size)) - } } impl Disk for DiskATAPI { + fn block_length(&mut self) -> u32 { + self.blk_size + } + fn size(&mut self) -> u64 { - match self.read_capacity() { - Ok((blk_count, blk_size)) => (blk_count as u64) * (blk_size as u64), - Err(_) => 0, // XXX - } + u64::from(self.blk_count) * u64::from(self.blk_size) } fn read(&mut self, block: u64, buffer: &mut [u8]) -> Result> { // TODO: Handle audio CDs, which use special READ CD command - let blk_len = self.block_length()?; + let blk_len = self.blk_size; let sectors = buffer.len() as u32 / blk_len; fn read10_cmd(block: u32, count: u16) -> [u8; 16] { @@ -147,8 +145,4 @@ impl Disk for DiskATAPI { fn write(&mut self, _block: u64, _buffer: &[u8]) -> Result> { Err(Error::new(EBADF)) // TODO: Implement writing } - - fn block_length(&mut self) -> Result { - Ok(self.read_capacity()?.1) - } } diff --git a/storage/ahcid/src/scheme.rs b/storage/ahcid/src/scheme.rs index 7ed30f2de1..4655529a6b 100644 --- a/storage/ahcid/src/scheme.rs +++ b/storage/ahcid/src/scheme.rs @@ -175,7 +175,7 @@ impl SchemeBlock for DiskScheme { let disk = self.disks.get_mut(number).ok_or(Error::new(EBADF))?; stat.st_mode = MODE_FILE; stat.st_size = disk.size(); - stat.st_blksize = disk.block_length()?; + stat.st_blksize = disk.block_length(); Ok(Some(0)) } Handle::Partition(disk_id, part_num) => { @@ -190,8 +190,8 @@ impl SchemeBlock for DiskScheme { }; stat.st_mode = MODE_FILE; // TODO: Block device? - stat.st_size = size * u64::from(disk.block_length()?); - stat.st_blksize = disk.block_length()?; + stat.st_size = size * u64::from(disk.block_length()); + stat.st_blksize = disk.block_length(); stat.st_blocks = size; Ok(Some(0)) } @@ -263,12 +263,12 @@ impl SchemeBlock for DiskScheme { } Handle::Disk(number) => { let disk = self.disks.get_mut(number).ok_or(Error::new(EBADF))?; - let blk_len = disk.block_length()?; + let blk_len = disk.block_length(); disk.read(offset / u64::from(blk_len), buf) } Handle::Partition(disk_num, part_num) => { let disk = self.disks.get_mut(disk_num).ok_or(Error::new(EBADF))?; - let blksize = disk.block_length()?; + let blksize = disk.block_length(); // validate that we're actually reading within the bounds of the partition let rel_block = offset / u64::from(blksize); @@ -304,12 +304,12 @@ 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_length()?; + let blk_len = disk.block_length(); disk.write(offset / u64::from(blk_len), buf) } Handle::Partition(disk_num, part_num) => { let disk = self.disks.get_mut(disk_num).ok_or(Error::new(EBADF))?; - let blksize = disk.block_length()?; + let blksize = disk.block_length(); // validate that we're actually reading within the bounds of the partition let rel_block = offset / u64::from(blksize); @@ -351,7 +351,7 @@ impl SchemeBlock for DiskScheme { .get(part_num as usize) .ok_or(Error::new(EBADF))? .size; - u64::from(disk.block_length()?) * block_count + u64::from(disk.block_length()) * block_count } }, )) diff --git a/storage/bcm2835-sdhcid/src/scheme.rs b/storage/bcm2835-sdhcid/src/scheme.rs index 70bf4772e0..7b5b195c61 100644 --- a/storage/bcm2835-sdhcid/src/scheme.rs +++ b/storage/bcm2835-sdhcid/src/scheme.rs @@ -164,7 +164,7 @@ impl SchemeBlock for DiskScheme { let disk = self.disks.get_mut(number).ok_or(Error::new(EBADF))?; stat.st_mode = MODE_FILE; stat.st_size = disk.size(); - stat.st_blksize = disk.block_length()?; + stat.st_blksize = disk.block_length(); Ok(Some(0)) } Handle::Partition(disk_id, part_num) => { @@ -179,8 +179,8 @@ impl SchemeBlock for DiskScheme { }; stat.st_mode = MODE_FILE; // TODO: Block device? - stat.st_size = size * u64::from(disk.block_length()?); - stat.st_blksize = disk.block_length()?; + stat.st_size = size * u64::from(disk.block_length()); + stat.st_blksize = disk.block_length(); stat.st_blocks = size; Ok(Some(0)) } @@ -251,12 +251,12 @@ impl SchemeBlock for DiskScheme { } Handle::Disk(number) => { let disk = self.disks.get_mut(number).ok_or(Error::new(EBADF))?; - let blk_len = disk.block_length()?; + let blk_len = disk.block_length(); disk.read(offset / u64::from(blk_len), buf) } Handle::Partition(disk_num, part_num) => { let disk = self.disks.get_mut(disk_num).ok_or(Error::new(EBADF))?; - let blksize = disk.block_length()?; + let blksize = disk.block_length(); // validate that we're actually reading within the bounds of the partition let rel_block = offset / u64::from(blksize); @@ -285,12 +285,12 @@ 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_length()?; + let blk_len = disk.block_length(); disk.write(offset / u64::from(blk_len), buf) } Handle::Partition(disk_num, part_num) => { let disk = self.disks.get_mut(disk_num).ok_or(Error::new(EBADF))?; - let blksize = disk.block_length()?; + let blksize = disk.block_length(); // validate that we're actually reading within the bounds of the partition let rel_block = offset / u64::from(blksize as u64); @@ -331,7 +331,7 @@ impl SchemeBlock for DiskScheme { .get(part_num as usize) .ok_or(Error::new(EBADF))? .size; - Ok(Some(u64::from(disk.block_length()?) * block_count)) + Ok(Some(u64::from(disk.block_length()) * block_count)) } } } diff --git a/storage/bcm2835-sdhcid/src/sd/mod.rs b/storage/bcm2835-sdhcid/src/sd/mod.rs index b371833773..deb3839f81 100644 --- a/storage/bcm2835-sdhcid/src/sd/mod.rs +++ b/storage/bcm2835-sdhcid/src/sd/mod.rs @@ -774,7 +774,7 @@ impl Disk for SdHostCtrl { } } - fn block_length(&mut self) -> Result { - Ok(512) + fn block_length(&mut self) -> u32 { + 512 } } diff --git a/storage/driver-block/src/lib.rs b/storage/driver-block/src/lib.rs index 02f771e475..24e45b8a1b 100644 --- a/storage/driver-block/src/lib.rs +++ b/storage/driver-block/src/lib.rs @@ -53,7 +53,7 @@ fn block_read( } pub trait Disk { - fn block_length(&mut self) -> syscall::error::Result; + fn block_length(&mut self) -> u32; fn size(&mut self) -> u64; fn read(&mut self, block: u64, buffer: &mut [u8]) -> syscall::Result>; @@ -68,8 +68,8 @@ pub struct DiskWrapper { impl DiskWrapper { pub fn pt(disk: &mut dyn Disk) -> Option { let bs = match disk.block_length() { - Ok(512) => LogicalBlockSize::Lb512, - Ok(4096) => LogicalBlockSize::Lb4096, + 512 => LogicalBlockSize::Lb512, + 4096 => LogicalBlockSize::Lb4096, _ => return None, }; struct Device<'a> { @@ -98,10 +98,7 @@ impl DiskWrapper { // TODO: Perhaps this impl should be used in the rest of the scheme. impl<'a> Read for Device<'a> { fn read(&mut self, buf: &mut [u8]) -> io::Result { - let blksize = self - .disk - .block_length() - .map_err(|err| io::Error::from_raw_os_error(err.errno))?; + let blksize = self.disk.block_length(); let size_in_blocks = self.disk.size() / u64::from(blksize); let disk = &mut self.disk; diff --git a/storage/ided/src/ide.rs b/storage/ided/src/ide.rs index b03213a464..94584a9dfb 100644 --- a/storage/ided/src/ide.rs +++ b/storage/ided/src/ide.rs @@ -461,7 +461,7 @@ impl Disk for AtaDisk { Ok(Some(count)) } - fn block_length(&mut self) -> Result { - Ok(512) + fn block_length(&mut self) -> u32 { + 512 } } diff --git a/storage/ided/src/scheme.rs b/storage/ided/src/scheme.rs index ff0a63e58a..aca709013e 100644 --- a/storage/ided/src/scheme.rs +++ b/storage/ided/src/scheme.rs @@ -178,7 +178,7 @@ impl SchemeBlock for DiskScheme { let disk = self.disks.get_mut(number).ok_or(Error::new(EBADF))?; stat.st_mode = MODE_FILE; stat.st_size = disk.size(); - stat.st_blksize = disk.block_length()?; + stat.st_blksize = disk.block_length(); Ok(Some(0)) } Handle::Partition(disk_id, part_num) => { @@ -193,8 +193,8 @@ impl SchemeBlock for DiskScheme { }; stat.st_mode = MODE_FILE; // TODO: Block device? - stat.st_size = size * u64::from(disk.block_length()?); - stat.st_blksize = disk.block_length()?; + stat.st_size = size * u64::from(disk.block_length()); + stat.st_blksize = disk.block_length(); stat.st_blocks = size; Ok(Some(0)) } @@ -265,12 +265,12 @@ impl SchemeBlock for DiskScheme { } Handle::Disk(number) => { let disk = self.disks.get_mut(number).ok_or(Error::new(EBADF))?; - let blk_len = disk.block_length()?; + let blk_len = disk.block_length(); disk.read(offset / u64::from(blk_len), buf) } Handle::Partition(disk_num, part_num) => { let disk = self.disks.get_mut(disk_num).ok_or(Error::new(EBADF))?; - let blksize = disk.block_length()?; + let blksize = disk.block_length(); // validate that we're actually reading within the bounds of the partition let rel_block = offset / u64::from(blksize); @@ -299,12 +299,12 @@ 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_length()?; + let blk_len = disk.block_length(); disk.write(offset / u64::from(blk_len), buf) } Handle::Partition(disk_num, part_num) => { let disk = self.disks.get_mut(disk_num).ok_or(Error::new(EBADF))?; - let blksize = disk.block_length()?; + let blksize = disk.block_length(); // validate that we're actually reading within the bounds of the partition let rel_block = offset / u64::from(blksize as u64); @@ -345,7 +345,7 @@ impl SchemeBlock for DiskScheme { .get(part_num as usize) .ok_or(Error::new(EBADF))? .size; - Ok(Some(u64::from(disk.block_length()?) * block_count)) + Ok(Some(u64::from(disk.block_length()) * block_count)) } } } diff --git a/storage/nvmed/src/scheme.rs b/storage/nvmed/src/scheme.rs index ee27527d9e..1a730920c9 100644 --- a/storage/nvmed/src/scheme.rs +++ b/storage/nvmed/src/scheme.rs @@ -24,8 +24,8 @@ enum Handle { struct NamespaceAndNvme<'a>(&'a Nvme, NvmeNamespace); impl Disk for NamespaceAndNvme<'_> { - fn block_length(&mut self) -> syscall::error::Result { - Ok(self.1.block_size.try_into().unwrap()) + fn block_length(&mut self) -> u32 { + self.1.block_size.try_into().unwrap() } fn size(&mut self) -> u64 { diff --git a/storage/virtio-blkd/src/scheme.rs b/storage/virtio-blkd/src/scheme.rs index d4727b9278..17d7ae62b5 100644 --- a/storage/virtio-blkd/src/scheme.rs +++ b/storage/virtio-blkd/src/scheme.rs @@ -124,8 +124,8 @@ impl<'a> DiskScheme<'a> { } impl driver_block::Disk for DiskScheme<'_> { - fn block_length(&mut self) -> syscall::error::Result { - Ok(self.cfg.block_size()) + fn block_length(&mut self) -> u32 { + self.cfg.block_size() } fn size(&mut self) -> u64 {