Use a safer approach to the HAL interface (#11)

* Expose address types rather than redefining them.

* Define a trait for HAL and pass it as a type parameter.

This avoids the need to use extern functions and removes some unsafe
blocks.

* Keep address and page count as usize.

Converting to u32 and back could lose bits.

Author: qwandor

examples/riscv/src/main.rs

@@ -10,6 +10,7 @@ use device_tree::util::SliceRead;
 use device_tree::{DeviceTree, Node};
 use log::{info, warn, LevelFilter};
 use virtio_drivers::*;
+use virtio_impl::HalImpl;
 
 mod virtio_impl;
 
@@ -72,7 +73,7 @@ fn virtio_probe(node: &Node) {
 }
 
 fn virtio_blk(header: &'static mut VirtIOHeader) {
-    let mut blk = VirtIOBlk::new(header).expect("failed to create blk driver");
+    let mut blk = VirtIOBlk::<HalImpl>::new(header).expect("failed to create blk driver");
     let mut input = vec![0xffu8; 512];
     let mut output = vec![0; 512];
     for i in 0..32 {
@@ -87,7 +88,7 @@ fn virtio_blk(header: &'static mut VirtIOHeader) {
 }
 
 fn virtio_gpu(header: &'static mut VirtIOHeader) {
-    let mut gpu = VirtIOGpu::new(header).expect("failed to create gpu driver");
+    let mut gpu = VirtIOGpu::<HalImpl>::new(header).expect("failed to create gpu driver");
     let fb = gpu.setup_framebuffer().expect("failed to get fb");
     for y in 0..768 {
         for x in 0..1024 {
@@ -103,7 +104,7 @@ fn virtio_gpu(header: &'static mut VirtIOHeader) {
 
 fn virtio_input(header: &'static mut VirtIOHeader) {
     //let mut event_buf = [0u64; 32];
-    let mut _input = VirtIOInput::new(header).expect("failed to create input driver");
+    let mut _input = VirtIOInput::<HalImpl>::new(header).expect("failed to create input driver");
     // loop {
     //     input.ack_interrupt().expect("failed to ack");
     //     info!("mouse: {:?}", input.mouse_xy());
@@ -112,7 +113,7 @@ fn virtio_input(header: &'static mut VirtIOHeader) {
 }
 
 fn virtio_net(header: &'static mut VirtIOHeader) {
-    let mut net = VirtIONet::new(header).expect("failed to create net driver");
+    let mut net = VirtIONet::<HalImpl>::new(header).expect("failed to create net driver");
     let mut buf = [0u8; 0x100];
     let len = net.recv(&mut buf).expect("failed to recv");
     info!("recv: {:?}", &buf[..len]);

examples/riscv/src/virtio_impl.rs

@@ -1,6 +1,7 @@
 use core::sync::atomic::*;
 use lazy_static::lazy_static;
 use log::trace;
+use virtio_drivers::{Hal, PhysAddr, VirtAddr};
 
 extern "C" {
     fn end();
@@ -10,28 +11,25 @@ lazy_static! {
     static ref DMA_PADDR: AtomicUsize = AtomicUsize::new(end as usize);
 }
 
-#[no_mangle]
-extern "C" fn virtio_dma_alloc(pages: usize) -> PhysAddr {
-    let paddr = DMA_PADDR.fetch_add(0x1000 * pages, Ordering::SeqCst);
-    trace!("alloc DMA: paddr={:#x}, pages={}", paddr, pages);
-    paddr
-}
+pub struct HalImpl;
 
-#[no_mangle]
-extern "C" fn virtio_dma_dealloc(paddr: PhysAddr, pages: usize) -> i32 {
-    trace!("dealloc DMA: paddr={:#x}, pages={}", paddr, pages);
-    0
-}
+impl Hal for HalImpl {
+    fn dma_alloc(pages: usize) -> PhysAddr {
+        let paddr = DMA_PADDR.fetch_add(0x1000 * pages, Ordering::SeqCst);
+        trace!("alloc DMA: paddr={:#x}, pages={}", paddr, pages);
+        paddr
+    }
 
-#[no_mangle]
-extern "C" fn virtio_phys_to_virt(paddr: PhysAddr) -> VirtAddr {
-    paddr
-}
+    fn dma_dealloc(paddr: PhysAddr, pages: usize) -> i32 {
+        trace!("dealloc DMA: paddr={:#x}, pages={}", paddr, pages);
+        0
+    }
 
-#[no_mangle]
-extern "C" fn virtio_virt_to_phys(vaddr: VirtAddr) -> PhysAddr {
-    vaddr
-}
+    fn phys_to_virt(paddr: PhysAddr) -> VirtAddr {
+        paddr
+    }
 
-type VirtAddr = usize;
-type PhysAddr = usize;
+    fn virt_to_phys(vaddr: VirtAddr) -> PhysAddr {
+        vaddr
+    }
+}

src/blk.rs

@@ -10,13 +10,13 @@ use volatile::Volatile;
 ///
 /// Read and write requests (and other exotic requests) are placed in the queue,
 /// and serviced (probably out of order) by the device except where noted.
-pub struct VirtIOBlk<'a> {
+pub struct VirtIOBlk<'a, H: Hal> {
     header: &'static mut VirtIOHeader,
-    queue: VirtQueue<'a>,
+    queue: VirtQueue<'a, H>,
     capacity: usize,
 }
 
-impl VirtIOBlk<'_> {
+impl<H: Hal> VirtIOBlk<'_, H> {
     /// Create a new VirtIO-Blk driver.
     pub fn new(header: &'static mut VirtIOHeader) -> Result<Self> {
         header.begin_init(|features| {

src/console.rs

@@ -10,17 +10,17 @@ const QUEUE_TRANSMITQ_PORT_0: usize = 1;
 
 /// Virtio console. Only one single port is allowed since ``alloc'' is disabled.
 /// Emergency and cols/rows unimplemented.
-pub struct VirtIOConsole<'a> {
+pub struct VirtIOConsole<'a, H: Hal> {
     header: &'static mut VirtIOHeader,
-    receiveq: VirtQueue<'a>,
-    transmitq: VirtQueue<'a>,
-    queue_buf_dma: DMA,
+    receiveq: VirtQueue<'a, H>,
+    transmitq: VirtQueue<'a, H>,
+    queue_buf_dma: DMA<H>,
     queue_buf_rx: &'a mut [u8],
     cursor: usize,
     pending_len: usize,
 }
 
-impl<'a> VirtIOConsole<'a> {
+impl<H: Hal> VirtIOConsole<'_, H> {
     /// Create a new VirtIO-Console driver.
     pub fn new(header: &'static mut VirtIOHeader) -> Result<Self> {
         header.begin_init(|features| {

src/gpu.rs

@@ -12,26 +12,26 @@ use volatile::{ReadOnly, Volatile, WriteOnly};
 /// a gpu with 3D support on the host machine.
 /// In 2D mode the virtio-gpu device provides support for ARGB Hardware cursors
 /// and multiple scanouts (aka heads).
-pub struct VirtIOGpu<'a> {
+pub struct VirtIOGpu<'a, H: Hal> {
     header: &'static mut VirtIOHeader,
     rect: Rect,
     /// DMA area of frame buffer.
-    frame_buffer_dma: Option<DMA>,
+    frame_buffer_dma: Option<DMA<H>>,
     /// DMA area of cursor image buffer.
-    cursor_buffer_dma: Option<DMA>,
+    cursor_buffer_dma: Option<DMA<H>>,
     /// Queue for sending control commands.
-    control_queue: VirtQueue<'a>,
+    control_queue: VirtQueue<'a, H>,
     /// Queue for sending cursor commands.
-    cursor_queue: VirtQueue<'a>,
+    cursor_queue: VirtQueue<'a, H>,
     /// Queue buffer DMA
-    queue_buf_dma: DMA,
+    queue_buf_dma: DMA<H>,
     /// Send buffer for queue.
     queue_buf_send: &'a mut [u8],
     /// Recv buffer for queue.
     queue_buf_recv: &'a mut [u8],
 }
 
-impl VirtIOGpu<'_> {
+impl<H: Hal> VirtIOGpu<'_, H> {
     /// Create a new VirtIO-Gpu driver.
     pub fn new(header: &'static mut VirtIOHeader) -> Result<Self> {
         header.begin_init(|features| {
@@ -153,9 +153,7 @@ impl VirtIOGpu<'_> {
         self.update_cursor(RESOURCE_ID_CURSOR, SCANOUT_ID, pos_x, pos_y, 0, 0, true)?;
         Ok(())
     }
-}
 
-impl VirtIOGpu<'_> {
     /// Send a request to the device and block for a response.
     fn request<Req, Rsp>(&mut self, req: Req) -> Result<Rsp> {
         unsafe {

src/hal.rs

@@ -1,37 +1,43 @@
 use super::*;
+use core::marker::PhantomData;
 
-type VirtAddr = usize;
-type PhysAddr = usize;
+/// A virtual memory address in the address space of the program.
+pub type VirtAddr = usize;
+
+/// A physical address as used for virtio.
+pub type PhysAddr = usize;
 
 /// A region of contiguous physical memory used for DMA.
-pub struct DMA {
-    paddr: u32,
-    pages: u32,
+pub struct DMA<H: Hal> {
+    paddr: usize,
+    pages: usize,
+    _phantom: PhantomData<H>,
 }
 
-impl DMA {
+impl<H: Hal> DMA<H> {
     pub fn new(pages: usize) -> Result<Self> {
-        let paddr = unsafe { virtio_dma_alloc(pages) };
+        let paddr = H::dma_alloc(pages);
         if paddr == 0 {
             return Err(Error::DmaError);
         }
         Ok(DMA {
-            paddr: paddr as u32,
-            pages: pages as u32,
+            paddr,
+            pages,
+            _phantom: PhantomData::default(),
         })
     }
 
     pub fn paddr(&self) -> usize {
-        self.paddr as usize
+        self.paddr
     }
 
     pub fn vaddr(&self) -> usize {
-        phys_to_virt(self.paddr as usize)
+        H::phys_to_virt(self.paddr)
     }
 
     /// Returns the physical page frame number.
     pub fn pfn(&self) -> u32 {
-        self.paddr >> 12
+        (self.paddr >> 12) as u32
     }
 
     /// Convert to a buffer
@@ -40,24 +46,23 @@ impl DMA {
     }
 }
 
-impl Drop for DMA {
+impl<H: Hal> Drop for DMA<H> {
     fn drop(&mut self) {
-        let err = unsafe { virtio_dma_dealloc(self.paddr as usize, self.pages as usize) };
+        let err = H::dma_dealloc(self.paddr as usize, self.pages as usize);
         assert_eq!(err, 0, "failed to deallocate DMA");
     }
 }
 
-pub fn phys_to_virt(paddr: PhysAddr) -> VirtAddr {
-    unsafe { virtio_phys_to_virt(paddr) }
-}
-
-pub fn virt_to_phys(vaddr: VirtAddr) -> PhysAddr {
-    unsafe { virtio_virt_to_phys(vaddr) }
-}
-
-extern "C" {
-    fn virtio_dma_alloc(pages: usize) -> PhysAddr;
-    fn virtio_dma_dealloc(paddr: PhysAddr, pages: usize) -> i32;
-    fn virtio_phys_to_virt(paddr: PhysAddr) -> VirtAddr;
-    fn virtio_virt_to_phys(vaddr: VirtAddr) -> PhysAddr;
+/// The interface which a particular hardware implementation must implement.
+pub trait Hal {
+    /// Allocates the given number of contiguous physical pages of DMA memory for virtio use.
+    fn dma_alloc(pages: usize) -> PhysAddr;
+    /// Deallocates the given contiguous physical DMA memory pages.
+    fn dma_dealloc(paddr: PhysAddr, pages: usize) -> i32;
+    /// Converts a physical address used for virtio to a virtual address which the program can
+    /// access.
+    fn phys_to_virt(paddr: PhysAddr) -> VirtAddr;
+    /// Converts a virtual address which the program can access to the corresponding physical
+    /// address to use for virtio.
+    fn virt_to_phys(vaddr: VirtAddr) -> PhysAddr;
 }

src/input.rs

@@ -9,14 +9,14 @@ use volatile::{ReadOnly, WriteOnly};
 /// An instance of the virtio device represents one such input device.
 /// Device behavior mirrors that of the evdev layer in Linux,
 /// making pass-through implementations on top of evdev easy.
-pub struct VirtIOInput<'a> {
+pub struct VirtIOInput<'a, H: Hal> {
     header: &'static mut VirtIOHeader,
-    event_queue: VirtQueue<'a>,
-    status_queue: VirtQueue<'a>,
+    event_queue: VirtQueue<'a, H>,
+    status_queue: VirtQueue<'a, H>,
     event_buf: Box<[InputEvent; 32]>,
 }
 
-impl<'a> VirtIOInput<'a> {
+impl<'a, H: Hal> VirtIOInput<'a, H> {
     /// Create a new VirtIO-Input driver.
     pub fn new(header: &'static mut VirtIOHeader) -> Result<Self> {
         let mut event_buf = Box::new([InputEvent::default(); QUEUE_SIZE]);

src/lib.rs

@@ -19,6 +19,7 @@ mod queue;
 pub use self::blk::{BlkResp, RespStatus, VirtIOBlk};
 pub use self::console::VirtIOConsole;
 pub use self::gpu::VirtIOGpu;
+pub use self::hal::{Hal, PhysAddr, VirtAddr};
 pub use self::header::*;
 pub use self::input::{InputConfigSelect, InputEvent, VirtIOInput};
 pub use self::net::VirtIONet;

src/net.rs

@@ -13,14 +13,14 @@ use volatile::{ReadOnly, Volatile};
 /// Empty buffers are placed in one virtqueue for receiving packets, and
 /// outgoing packets are enqueued into another for transmission in that order.
 /// A third command queue is used to control advanced filtering features.
-pub struct VirtIONet<'a> {
+pub struct VirtIONet<'a, H: Hal> {
     header: &'static mut VirtIOHeader,
     mac: EthernetAddress,
-    recv_queue: VirtQueue<'a>,
-    send_queue: VirtQueue<'a>,
+    recv_queue: VirtQueue<'a, H>,
+    send_queue: VirtQueue<'a, H>,
 }
 
-impl VirtIONet<'_> {
+impl<H: Hal> VirtIONet<'_, H> {
     /// Create a new VirtIO-Net driver.
     pub fn new(header: &'static mut VirtIOHeader) -> Result<Self> {
         header.begin_init(|features| {

src/queue.rs

@@ -12,9 +12,9 @@ use volatile::Volatile;
 ///
 /// Each device can have zero or more virtqueues.
 #[repr(C)]
-pub struct VirtQueue<'a> {
+pub struct VirtQueue<'a, H: Hal> {
     /// DMA guard
-    dma: DMA,
+    dma: DMA<H>,
     /// Descriptor table
     desc: &'a mut [Descriptor],
     /// Available ring
@@ -34,7 +34,7 @@ pub struct VirtQueue<'a> {
     last_used_idx: u16,
 }
 
-impl VirtQueue<'_> {
+impl<H: Hal> VirtQueue<'_, H> {
     /// Create a new VirtQueue.
     pub fn new(header: &mut VirtIOHeader, idx: usize, size: u16) -> Result<Self> {
         if header.queue_used(idx as u32) {
@@ -89,14 +89,14 @@ impl VirtQueue<'_> {
         let mut last = self.free_head;
         for input in inputs.iter() {
             let desc = &mut self.desc[self.free_head as usize];
-            desc.set_buf(input);
+            desc.set_buf::<H>(input);
             desc.flags.write(DescFlags::NEXT);
             last = self.free_head;
             self.free_head = desc.next.read();
         }
         for output in outputs.iter() {
             let desc = &mut self.desc[self.free_head as usize];
-            desc.set_buf(output);
+            desc.set_buf::<H>(output);
             desc.flags.write(DescFlags::NEXT | DescFlags::WRITE);
             last = self.free_head;
             self.free_head = desc.next.read();
@@ -214,8 +214,9 @@ struct Descriptor {
 }
 
 impl Descriptor {
-    fn set_buf(&mut self, buf: &[u8]) {
-        self.addr.write(virt_to_phys(buf.as_ptr() as usize) as u64);
+    fn set_buf<H: Hal>(&mut self, buf: &[u8]) {
+        self.addr
+            .write(H::virt_to_phys(buf.as_ptr() as usize) as u64);
         self.len.write(buf.len() as u32);
     }
 }