From 65c431756430c9f00b7da7afbc43b5a8fb02a4fc Mon Sep 17 00:00:00 2001 From: Anhad Singh Date: Fri, 23 Jun 2023 16:18:14 +1000 Subject: [PATCH] virtiod: refactoring Signed-off-by: Anhad Singh --- virtiod/src/main.rs | 33 ++++---- virtiod/src/scheme.rs | 171 ++++++++++++++------------------------- virtiod/src/transport.rs | 17 +++- 3 files changed, 94 insertions(+), 127 deletions(-) diff --git a/virtiod/src/main.rs b/virtiod/src/main.rs index 3599fee16c..c965301fc1 100644 --- a/virtiod/src/main.rs +++ b/virtiod/src/main.rs @@ -3,7 +3,7 @@ use core::ptr::NonNull; use std::fs::File; -use std::io::{self, ErrorKind}; +use std::io; use std::io::{Read, Write}; use std::ops::{Add, Div, Rem}; use std::os::fd::{AsRawFd, FromRawFd, RawFd}; @@ -375,7 +375,8 @@ fn deamon(deamon: redox_daemon::Daemon) -> anyhow::Result<()> { .add( irq_handle.as_raw_fd(), move |_| -> Result, io::Error> { - let isr = isr.get() as usize; + // Read from ISR to acknowledge the interrupt. + let _isr = isr.get() as usize; let mut inner = queue_copy.inner.lock().unwrap(); let used_head = inner.used.head_index(); @@ -384,18 +385,20 @@ fn deamon(deamon: redox_daemon::Daemon) -> anyhow::Result<()> { return Ok(None); } - let used = inner.used.get_element_at((used_head - 1) as usize); - let mut desc_idx = used.table_index.get(); - inner.descriptor_stack.push_back(desc_idx as u16); - - loop { - let desc = &inner.descriptor[desc_idx as usize]; - if !desc.flags.contains(DescriptorFlags::NEXT) { - break; - } - - desc_idx = desc.next.into(); + for i in progress_head..used_head { + let used = inner.used.get_element_at(i as usize); + let mut desc_idx = used.table_index.get(); inner.descriptor_stack.push_back(desc_idx as u16); + + loop { + let desc = &inner.descriptor[desc_idx as usize]; + if !desc.flags.contains(DescriptorFlags::NEXT) { + break; + } + + desc_idx = desc.next.into(); + inner.descriptor_stack.push_back(desc_idx as u16); + } } progress_head = used_head; @@ -405,7 +408,7 @@ fn deamon(deamon: redox_daemon::Daemon) -> anyhow::Result<()> { irq_handle.read(&mut buf)?; // Acknowledge the interrupt. // irq_handle.write(&buf)?; - Ok(Some(isr)) + Ok(None) }, ) .unwrap(); @@ -437,7 +440,7 @@ fn deamon(deamon: redox_daemon::Daemon) -> anyhow::Result<()> { let mut socket_file = unsafe { File::from_raw_fd(socket_fd as RawFd) }; - let mut scheme = scheme::DiskScheme::new(scheme_name, queue, device_space); + let mut scheme = scheme::DiskScheme::new(queue, device_space); deamon.ready().expect("virtio: failed to deamonize"); diff --git a/virtiod/src/scheme.rs b/virtiod/src/scheme.rs index 8bdd0717e0..ad8b410766 100644 --- a/virtiod/src/scheme.rs +++ b/virtiod/src/scheme.rs @@ -24,6 +24,10 @@ const BLK_SIZE: u64 = 512; trait BlkExtension { /// XXX: Reads only one block despite the size of the output buffer. Use [`BlkExtension::read`] instead. fn read_block(&self, block: u64, block_bytes: &mut [u8]) -> usize; + + /// XXX: Reads only one block despite the size of the output buffer. Use [`BlkExtension::write`] instead. + fn write_block(&self, block: u64, block_bytes: &[u8]) -> usize; + fn read(&self, block: u64, block_bytes: &mut [u8]) -> usize { let sectors = block_bytes.len() / BLK_SIZE as usize; @@ -31,6 +35,14 @@ trait BlkExtension { .map(|i| self.read_block(block + i as u64, &mut block_bytes[i * BLK_SIZE as usize..])) .sum() } + + fn write(&self, block: u64, block_bytes: &[u8]) -> usize { + let sectors = block_bytes.len() / BLK_SIZE as usize; + + (0..sectors) + .map(|i| self.write_block(block + i as u64, &block_bytes[i * BLK_SIZE as usize..])) + .sum() + } } impl BlkExtension for Queue<'_> { @@ -64,6 +76,36 @@ impl BlkExtension for Queue<'_> { size } + + fn write_block(&self, block: u64, block_bytes: &[u8]) -> usize { + let req = syscall::Dma::new(BlockVirtRequest { + ty: BlockRequestTy::Out, + reserved: 0, + sector: block, + }) + .unwrap(); + + let mut result = syscall::Dma::new([0u8; 512]).unwrap(); + result.copy_from_slice(&block_bytes[..512]); + + let status = syscall::Dma::new(u8::MAX).unwrap(); + + let chain = ChainBuilder::new() + .chain(Buffer::new(&req).flags(DescriptorFlags::NEXT)) + .chain(Buffer::new(&result).flags(DescriptorFlags::NEXT)) + .chain(Buffer::new(&status).flags(DescriptorFlags::WRITE_ONLY)) + .build(); + + self.send(chain); + + // FIXME: interrupts are for a reason + while *status != 0 { + core::hint::spin_loop(); + } + + std::thread::yield_now(); + block_bytes.len() + } } pub enum Handle { @@ -85,7 +127,6 @@ pub enum Handle { } pub struct DiskScheme<'a> { - name: String, queue: Arc>, next_id: usize, cfg: &'a mut BlockDeviceConfig, @@ -94,9 +135,8 @@ pub struct DiskScheme<'a> { } impl<'a> DiskScheme<'a> { - pub fn new(name: String, queue: Arc>, cfg: &'a mut BlockDeviceConfig) -> Self { + pub fn new(queue: Arc>, cfg: &'a mut BlockDeviceConfig) -> Self { let mut this = Self { - name, queue, next_id: 0, cfg, @@ -197,10 +237,10 @@ impl<'a> SchemeBlockMut for DiskScheme<'a> { &mut self, path: &str, flags: usize, - uid: u32, - gid: u32, + _uid: u32, + _gid: u32, ) -> syscall::Result> { - log::info!("virtioo: open: {}", path); + log::info!("virtiod: open: {}", path); let path_str = path.trim_matches('/'); if path_str.is_empty() { @@ -264,28 +304,6 @@ impl<'a> SchemeBlockMut for DiskScheme<'a> { } } - fn chmod( - &mut self, - path: &str, - mode: u16, - uid: u32, - gid: u32, - ) -> syscall::Result> { - todo!() - } - - fn rmdir(&mut self, path: &str, uid: u32, gid: u32) -> syscall::Result> { - todo!() - } - - fn unlink(&mut self, path: &str, uid: u32, gid: u32) -> syscall::Result> { - todo!() - } - - fn dup(&mut self, old_id: usize, buf: &[u8]) -> syscall::Result> { - todo!() - } - fn read(&mut self, id: usize, buf: &mut [u8]) -> syscall::Result> { match *self.handles.get_mut(&id).ok_or(Error::new(EBADF))? { Handle::List { @@ -323,7 +341,7 @@ impl<'a> SchemeBlockMut for DiskScheme<'a> { Handle::Disk { ref mut offset } => { let block_size = self.cfg.block_size(); - let count = self.queue.read((*offset as u64) / 512, buf); + let count = self.queue.read((*offset as u64) / block_size as u64, buf); *offset += count; Ok(Some(count)) } @@ -331,8 +349,17 @@ impl<'a> SchemeBlockMut for DiskScheme<'a> { } fn write(&mut self, id: usize, buf: &[u8]) -> syscall::Result> { - // lets assume this worked - Ok(Some(buf.len())) + match *self.handles.get_mut(&id).ok_or(Error::new(EBADF))? { + Handle::Disk { ref mut offset } => { + let block_size = self.cfg.block_size(); + let count = self.queue.write((*offset as u64) / block_size as u64, buf); + + *offset += count; + Ok(Some(count)) + } + + _ => unimplemented!(), + } } fn seek(&mut self, id: usize, pos: isize, whence: usize) -> syscall::Result> { @@ -394,91 +421,15 @@ impl<'a> SchemeBlockMut for DiskScheme<'a> { } } - fn fchmod(&mut self, id: usize, mode: u16) -> syscall::Result> { + fn fpath(&mut self, _id: usize, _buf: &mut [u8]) -> syscall::Result> { todo!() } - fn fchown(&mut self, id: usize, uid: u32, gid: u32) -> syscall::Result> { + fn fstat(&mut self, _id: usize, _stat: &mut syscall::Stat) -> syscall::Result> { todo!() } - fn fcntl(&mut self, id: usize, cmd: usize, arg: usize) -> syscall::Result> { - todo!() - } - - fn fevent( - &mut self, - id: usize, - flags: syscall::EventFlags, - ) -> syscall::Result> { - todo!() - } - - fn fmap_old(&mut self, id: usize, map: &syscall::OldMap) -> syscall::Result> { - todo!() - } - - fn fmap(&mut self, id: usize, map: &syscall::Map) -> syscall::Result> { - if map.flags.contains(syscall::MapFlags::MAP_FIXED) { - return Err(syscall::Error::new(syscall::EINVAL)); - } - self.fmap_old( - id, - &syscall::OldMap { - offset: map.offset, - size: map.size, - flags: map.flags, - }, - ) - } - - fn funmap_old(&mut self, address: usize) -> syscall::Result> { - todo!() - } - - fn funmap(&mut self, address: usize, length: usize) -> syscall::Result> { - todo!() - } - - fn fpath(&mut self, id: usize, buf: &mut [u8]) -> syscall::Result> { - todo!() - } - - fn frename( - &mut self, - id: usize, - path: &str, - uid: u32, - gid: u32, - ) -> syscall::Result> { - todo!() - } - - fn fstat(&mut self, id: usize, stat: &mut syscall::Stat) -> syscall::Result> { - todo!() - } - - fn fstatvfs( - &mut self, - id: usize, - stat: &mut syscall::StatVfs, - ) -> syscall::Result> { - todo!() - } - - fn fsync(&mut self, id: usize) -> syscall::Result> { - todo!() - } - - fn ftruncate(&mut self, id: usize, len: usize) -> syscall::Result> { - todo!() - } - - fn futimens( - &mut self, - id: usize, - times: &[syscall::TimeSpec], - ) -> syscall::Result> { + fn dup(&mut self, _old_id: usize, _buf: &[u8]) -> Result> { todo!() } diff --git a/virtiod/src/transport.rs b/virtiod/src/transport.rs index 708389cf0c..21302b9d53 100644 --- a/virtiod/src/transport.rs +++ b/virtiod/src/transport.rs @@ -136,10 +136,23 @@ impl<'a> Queue<'a> { } fn alloc_descriptor(&self) -> usize { - if let Some(index) = self.inner.lock().unwrap().descriptor_stack.pop_front() { + let mut inner = self.inner.lock().unwrap(); + + if let Some(index) = inner.descriptor_stack.pop_front() { index as usize } else { - panic!("virtio: descriptor exhaustion"); + log::warn!("virtiod: descriptors exhausted, waiting on garabage collector"); + drop(inner); + + // Wait for the garbage collector thread to release some descriptors. + // + // TODO(andypython): Instead of just yielding, we should have a proper notificiation + // mechanism. I am not aware whats the standard way redox applications + // or drivers implement basically a WaitQueue which you can use to wake + // up a thread. The descripts really should NEVER run out, but if they + // do, have a proper way to handle them. + std::thread::yield_now(); + self.alloc_descriptor() } } }