Merge pull request #48 from rcore-os/dmacleanup

Clean up Dma

src/device/console.rs

@@ -75,7 +75,12 @@ impl<H: Hal, T: Transport> VirtIOConsole<'_, H, T> {
         let receiveq = VirtQueue::new(&mut transport, QUEUE_RECEIVEQ_PORT_0, QUEUE_SIZE)?;
         let transmitq = VirtQueue::new(&mut transport, QUEUE_TRANSMITQ_PORT_0, QUEUE_SIZE)?;
         let queue_buf_dma = Dma::new(1, BufferDirection::DeviceToDriver)?;
-        let queue_buf_rx = unsafe { &mut queue_buf_dma.as_buf()[0..] };
+
+        // Safe because no alignment or initialisation is required for [u8], the DMA buffer is
+        // dereferenceable, and the lifetime of the reference matches the lifetime of the DMA buffer
+        // (which we don't otherwise access).
+        let queue_buf_rx = unsafe { queue_buf_dma.raw_slice().as_mut() };
+
         transport.finish_init();
         let mut console = VirtIOConsole {
             transport,

src/device/gpu.rs

@@ -62,8 +62,8 @@ impl<H: Hal, T: Transport> VirtIOGpu<'_, H, T> {
 
         let dma_send = Dma::new(1, BufferDirection::DriverToDevice)?;
         let dma_recv = Dma::new(1, BufferDirection::DeviceToDriver)?;
-        let queue_buf_send = unsafe { dma_send.as_buf() };
-        let queue_buf_recv = unsafe { dma_recv.as_buf() };
+        let queue_buf_send = unsafe { dma_send.raw_slice().as_mut() };
+        let queue_buf_recv = unsafe { dma_recv.raw_slice().as_mut() };
 
         transport.finish_init();
 
@@ -116,7 +116,7 @@ impl<H: Hal, T: Transport> VirtIOGpu<'_, H, T> {
         // map frame buffer to screen
         self.set_scanout(display_info.rect, SCANOUT_ID, RESOURCE_ID_FB)?;
 
-        let buf = unsafe { frame_buffer_dma.as_buf() };
+        let buf = unsafe { frame_buffer_dma.raw_slice().as_mut() };
         self.frame_buffer_dma = Some(frame_buffer_dma);
         Ok(buf)
     }
@@ -145,7 +145,7 @@ impl<H: Hal, T: Transport> VirtIOGpu<'_, H, T> {
             return Err(Error::InvalidParam);
         }
         let cursor_buffer_dma = Dma::new(pages(size as usize), BufferDirection::DriverToDevice)?;
-        let buf = unsafe { cursor_buffer_dma.as_buf() };
+        let buf = unsafe { cursor_buffer_dma.raw_slice().as_mut() };
         buf.copy_from_slice(cursor_image);
 
         self.resource_create_2d(RESOURCE_ID_CURSOR, CURSOR_RECT.width, CURSOR_RECT.height)?;

src/hal.rs

@@ -19,6 +19,8 @@ pub struct Dma<H: Hal> {
 }
 
 impl<H: Hal> Dma<H> {
+    /// Allocates the given number of pages of physically contiguous memory to be used for DMA in
+    /// the given direction.
     pub fn new(pages: usize, direction: BufferDirection) -> Result<Self> {
         let paddr = H::dma_alloc(pages, direction);
         if paddr == 0 {
@@ -31,17 +33,22 @@ impl<H: Hal> Dma<H> {
         })
     }
 
+    /// Returns the physical address of the start of the DMA region, as seen by devices.
     pub fn paddr(&self) -> usize {
         self.paddr
     }
 
-    pub fn vaddr(&self) -> usize {
-        H::phys_to_virt(self.paddr)
+    /// Returns a pointer to the given offset within the DMA region.
+    pub fn vaddr(&self, offset: usize) -> NonNull<u8> {
+        assert!(offset < self.pages * PAGE_SIZE);
+        NonNull::new((H::phys_to_virt(self.paddr) + offset) as _).unwrap()
     }
 
-    /// Convert to a buffer
-    pub unsafe fn as_buf(&self) -> &'static mut [u8] {
-        core::slice::from_raw_parts_mut(self.vaddr() as _, PAGE_SIZE * self.pages)
+    /// Returns a pointer to the entire DMA region as a slice.
+    pub fn raw_slice(&self) -> NonNull<[u8]> {
+        let raw_slice =
+            core::ptr::slice_from_raw_parts_mut(self.vaddr(0).as_ptr(), self.pages * PAGE_SIZE);
+        NonNull::new(raw_slice).unwrap()
     }
 }
 

src/lib.rs

@@ -53,6 +53,8 @@ mod queue;
 pub mod transport;
 mod volatile;
 
+use core::ptr::{self, NonNull};
+
 pub use self::hal::{BufferDirection, Hal, PhysAddr, VirtAddr};
 
 /// The page size in bytes supported by the library (4 KiB).
@@ -95,3 +97,9 @@ fn align_up(size: usize) -> usize {
 fn pages(size: usize) -> usize {
     (size + PAGE_SIZE - 1) / PAGE_SIZE
 }
+
+// TODO: Use NonNull::slice_from_raw_parts once it is stable.
+/// Creates a non-null raw slice from a non-null thin pointer and length.
+fn nonnull_slice_from_raw_parts<T>(data: NonNull<T>, len: usize) -> NonNull<[T]> {
+    NonNull::new(ptr::slice_from_raw_parts_mut(data.as_ptr(), len)).unwrap()
+}

src/queue.rs

@@ -1,12 +1,16 @@
-use crate::hal::{BufferDirection, Dma, Hal, PhysAddr, VirtAddr};
+#[cfg(test)]
+use crate::hal::VirtAddr;
+use crate::hal::{BufferDirection, Dma, Hal, PhysAddr};
 use crate::transport::Transport;
-use crate::{align_up, pages, Error, Result, PAGE_SIZE};
+use crate::{align_up, nonnull_slice_from_raw_parts, pages, Error, Result, PAGE_SIZE};
 use bitflags::bitflags;
 #[cfg(test)]
 use core::cmp::min;
 use core::hint::spin_loop;
 use core::mem::size_of;
-use core::ptr::{self, addr_of_mut, NonNull};
+#[cfg(test)]
+use core::ptr;
+use core::ptr::{addr_of_mut, NonNull};
 use core::sync::atomic::{fence, Ordering};
 
 /// The mechanism for bulk data transport on virtio devices.
@@ -62,13 +66,12 @@ impl<H: Hal> VirtQueue<H> {
             layout.device_area_paddr(),
         );
 
-        let desc = NonNull::new(ptr::slice_from_raw_parts_mut(
-            layout.descriptors_vaddr() as *mut Descriptor,
+        let desc = nonnull_slice_from_raw_parts(
+            layout.descriptors_vaddr().cast::<Descriptor>(),
             size as usize,
-        ))
-        .unwrap();
-        let avail = NonNull::new(layout.avail_vaddr() as *mut AvailRing).unwrap();
-        let used = NonNull::new(layout.used_vaddr() as *mut UsedRing).unwrap();
+        );
+        let avail = layout.avail_vaddr().cast();
+        let used = layout.used_vaddr().cast();
 
         // Link descriptors together.
         for i in 0..(size - 1) {
@@ -352,14 +355,14 @@ impl<H: Hal> VirtQueueLayout<H> {
         }
     }
 
-    /// Returns the virtual address of the descriptor table (in the descriptor area).
-    fn descriptors_vaddr(&self) -> VirtAddr {
+    /// Returns a pointer to the descriptor table (in the descriptor area).
+    fn descriptors_vaddr(&self) -> NonNull<u8> {
         match self {
-            Self::Legacy { dma, .. } => dma.vaddr(),
+            Self::Legacy { dma, .. } => dma.vaddr(0),
             Self::Modern {
                 driver_to_device_dma,
                 ..
-            } => driver_to_device_dma.vaddr(),
+            } => driver_to_device_dma.vaddr(0),
         }
     }
 
@@ -377,17 +380,17 @@ impl<H: Hal> VirtQueueLayout<H> {
         }
     }
 
-    /// Returns the virtual address of the available ring (in the driver area).
-    fn avail_vaddr(&self) -> VirtAddr {
+    /// Returns a pointer to the available ring (in the driver area).
+    fn avail_vaddr(&self) -> NonNull<u8> {
         match self {
             Self::Legacy {
                 dma, avail_offset, ..
-            } => dma.vaddr() + avail_offset,
+            } => dma.vaddr(*avail_offset),
             Self::Modern {
                 driver_to_device_dma,
                 avail_offset,
                 ..
-            } => driver_to_device_dma.vaddr() + avail_offset,
+            } => driver_to_device_dma.vaddr(*avail_offset),
         }
     }
 
@@ -404,16 +407,16 @@ impl<H: Hal> VirtQueueLayout<H> {
         }
     }
 
-    /// Returns the virtual address of the used ring (in the driver area).
-    fn used_vaddr(&self) -> VirtAddr {
+    /// Returns a pointer to the used ring (in the driver area).
+    fn used_vaddr(&self) -> NonNull<u8> {
         match self {
             Self::Legacy {
                 dma, used_offset, ..
-            } => dma.vaddr() + used_offset,
+            } => dma.vaddr(*used_offset),
             Self::Modern {
                 device_to_driver_dma,
                 ..
-            } => device_to_driver_dma.vaddr(),
+            } => device_to_driver_dma.vaddr(0),
         }
     }
 }

src/transport/pci.rs

@@ -6,6 +6,7 @@ use self::bus::{DeviceFunction, DeviceFunctionInfo, PciError, PciRoot, PCI_CAP_I
 use super::{DeviceStatus, DeviceType, Transport};
 use crate::{
     hal::{Hal, PhysAddr, VirtAddr},
+    nonnull_slice_from_raw_parts,
     volatile::{
         volread, volwrite, ReadOnly, Volatile, VolatileReadable, VolatileWritable, WriteOnly,
     },
@@ -14,7 +15,7 @@ use crate::{
 use core::{
     fmt::{self, Display, Formatter},
     mem::{align_of, size_of},
-    ptr::{self, addr_of_mut, NonNull},
+    ptr::{addr_of_mut, NonNull},
 };
 
 /// The PCI vendor ID for VirtIO devices.
@@ -403,9 +404,10 @@ fn get_bar_region_slice<H: Hal, T>(
     struct_info: &VirtioCapabilityInfo,
 ) -> Result<NonNull<[T]>, VirtioPciError> {
     let ptr = get_bar_region::<H, T>(root, device_function, struct_info)?;
-    let raw_slice =
-        ptr::slice_from_raw_parts_mut(ptr.as_ptr(), struct_info.length as usize / size_of::<T>());
-    Ok(NonNull::new(raw_slice).unwrap())
+    Ok(nonnull_slice_from_raw_parts(
+        ptr,
+        struct_info.length as usize / size_of::<T>(),
+    ))
 }
 
 /// An error encountered initialising a VirtIO PCI transport.