Merge pull request #46 from rcore-os/dmaregions

Specify direction when allocating DMA regions

examples/aarch64/src/hal.rs

@@ -18,7 +18,7 @@ lazy_static! {
 pub struct HalImpl;
 
 impl Hal for HalImpl {
-    fn dma_alloc(pages: usize) -> PhysAddr {
+    fn dma_alloc(pages: usize, _direction: BufferDirection) -> PhysAddr {
         let paddr = DMA_PADDR.fetch_add(PAGE_SIZE * pages, Ordering::SeqCst);
         trace!("alloc DMA: paddr={:#x}, pages={}", paddr, pages);
         paddr

examples/riscv/src/virtio_impl.rs

@@ -17,7 +17,7 @@ lazy_static! {
 pub struct HalImpl;
 
 impl Hal for HalImpl {
-    fn dma_alloc(pages: usize) -> PhysAddr {
+    fn dma_alloc(pages: usize, _direction: BufferDirection) -> PhysAddr {
         let paddr = DMA_PADDR.fetch_add(PAGE_SIZE * pages, Ordering::SeqCst);
         trace!("alloc DMA: paddr={:#x}, pages={}", paddr, pages);
         paddr

src/device/console.rs

@@ -1,6 +1,6 @@
 //! Driver for VirtIO console devices.
 
-use crate::hal::{Dma, Hal};
+use crate::hal::{BufferDirection, Dma, Hal};
 use crate::queue::VirtQueue;
 use crate::transport::Transport;
 use crate::volatile::{volread, ReadOnly, WriteOnly};
@@ -74,7 +74,7 @@ impl<H: Hal, T: Transport> VirtIOConsole<'_, H, T> {
         let config_space = transport.config_space::<Config>()?;
         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)?;
+        let queue_buf_dma = Dma::new(1, BufferDirection::DeviceToDriver)?;
         let queue_buf_rx = unsafe { &mut queue_buf_dma.as_buf()[0..] };
         transport.finish_init();
         let mut console = VirtIOConsole {

src/device/gpu.rs

@@ -1,10 +1,10 @@
 //! Driver for VirtIO GPU devices.
 
-use crate::hal::{Dma, Hal};
+use crate::hal::{BufferDirection, Dma, Hal};
 use crate::queue::VirtQueue;
 use crate::transport::Transport;
 use crate::volatile::{volread, ReadOnly, Volatile, WriteOnly};
-use crate::{pages, Error, Result, PAGE_SIZE};
+use crate::{pages, Error, Result};
 use bitflags::bitflags;
 use log::info;
 
@@ -26,8 +26,10 @@ pub struct VirtIOGpu<'a, H: Hal, T: Transport> {
     control_queue: VirtQueue<H>,
     /// Queue for sending cursor commands.
     cursor_queue: VirtQueue<H>,
-    /// Queue buffer DMA
-    queue_buf_dma: Dma<H>,
+    /// DMA region for sending data to the device.
+    dma_send: Dma<H>,
+    /// DMA region for receiving data from the device.
+    dma_recv: Dma<H>,
     /// Send buffer for queue.
     queue_buf_send: &'a mut [u8],
     /// Recv buffer for queue.
@@ -58,9 +60,10 @@ impl<H: Hal, T: Transport> VirtIOGpu<'_, H, T> {
         let control_queue = VirtQueue::new(&mut transport, QUEUE_TRANSMIT, 2)?;
         let cursor_queue = VirtQueue::new(&mut transport, QUEUE_CURSOR, 2)?;
 
-        let queue_buf_dma = Dma::new(2)?;
-        let queue_buf_send = unsafe { &mut queue_buf_dma.as_buf()[..PAGE_SIZE] };
-        let queue_buf_recv = unsafe { &mut queue_buf_dma.as_buf()[PAGE_SIZE..] };
+        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() };
 
         transport.finish_init();
 
@@ -71,7 +74,8 @@ impl<H: Hal, T: Transport> VirtIOGpu<'_, H, T> {
             rect: None,
             control_queue,
             cursor_queue,
-            queue_buf_dma,
+            dma_send,
+            dma_recv,
             queue_buf_send,
             queue_buf_recv,
         })
@@ -104,7 +108,7 @@ impl<H: Hal, T: Transport> VirtIOGpu<'_, H, T> {
 
         // alloc continuous pages for the frame buffer
         let size = display_info.rect.width * display_info.rect.height * 4;
-        let frame_buffer_dma = Dma::new(pages(size as usize))?;
+        let frame_buffer_dma = Dma::new(pages(size as usize), BufferDirection::DriverToDevice)?;
 
         // resource_attach_backing
         self.resource_attach_backing(RESOURCE_ID_FB, frame_buffer_dma.paddr() as u64, size)?;
@@ -140,7 +144,7 @@ impl<H: Hal, T: Transport> VirtIOGpu<'_, H, T> {
         if cursor_image.len() != size as usize {
             return Err(Error::InvalidParam);
         }
-        let cursor_buffer_dma = Dma::new(pages(size as usize))?;
+        let cursor_buffer_dma = Dma::new(pages(size as usize), BufferDirection::DriverToDevice)?;
         let buf = unsafe { cursor_buffer_dma.as_buf() };
         buf.copy_from_slice(cursor_image);
 

src/hal.rs

@@ -19,8 +19,8 @@ pub struct Dma<H: Hal> {
 }
 
 impl<H: Hal> Dma<H> {
-    pub fn new(pages: usize) -> Result<Self> {
-        let paddr = H::dma_alloc(pages);
+    pub fn new(pages: usize, direction: BufferDirection) -> Result<Self> {
+        let paddr = H::dma_alloc(pages, direction);
         if paddr == 0 {
             return Err(Error::DmaError);
         }
@@ -55,11 +55,14 @@ impl<H: Hal> Drop for Dma<H> {
 /// 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;
+    fn dma_alloc(pages: usize, direction: BufferDirection) -> 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.
+    ///
+    /// This is used both for DMA regions allocated by `dma_alloc`, and for MMIO addresses within
+    /// BARs read from the device (for the PCI transport).
     fn phys_to_virt(paddr: PhysAddr) -> VirtAddr;
     /// Shares the given memory range with the device, and returns the physical address that the
     /// device can use to access it.
@@ -75,8 +78,10 @@ pub trait Hal {
 /// The direction in which a buffer is passed.
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
 pub enum BufferDirection {
-    /// The buffer is written by the driver and read by the device.
+    /// The buffer may be read or written by the driver, but only read by the device.
     DriverToDevice,
-    /// The buffer is written by the device and read by the driver.
+    /// The buffer may be read or written by the device, but only read by the driver.
     DeviceToDriver,
+    /// The buffer may be read or written by both the device and the driver.
+    Both,
 }

src/hal/fake.rs

@@ -9,7 +9,7 @@ pub struct FakeHal;
 
 /// Fake HAL implementation for use in unit tests.
 impl Hal for FakeHal {
-    fn dma_alloc(pages: usize) -> PhysAddr {
+    fn dma_alloc(pages: usize, _direction: BufferDirection) -> PhysAddr {
         assert_ne!(pages, 0);
         let layout = Layout::from_size_align(pages * PAGE_SIZE, PAGE_SIZE).unwrap();
         // Safe because the size and alignment of the layout are non-zero.

src/queue.rs

@@ -1,8 +1,6 @@
-#[cfg(test)]
-use crate::hal::VirtAddr;
-use crate::hal::{BufferDirection, Dma, Hal};
+use crate::hal::{BufferDirection, Dma, Hal, PhysAddr, VirtAddr};
 use crate::transport::Transport;
-use crate::{align_up, Error, Result, PAGE_SIZE};
+use crate::{align_up, pages, Error, Result, PAGE_SIZE};
 use bitflags::bitflags;
 #[cfg(test)]
 use core::cmp::min;
@@ -17,7 +15,7 @@ use core::sync::atomic::{fence, Ordering};
 #[derive(Debug)]
 pub struct VirtQueue<H: Hal> {
     /// DMA guard
-    dma: Dma<H>,
+    layout: VirtQueueLayout<H>,
     /// Descriptor table
     desc: NonNull<[Descriptor]>,
     /// Available ring
@@ -49,25 +47,28 @@ impl<H: Hal> VirtQueue<H> {
         if !size.is_power_of_two() || transport.max_queue_size() < size as u32 {
             return Err(Error::InvalidParam);
         }
-        let layout = VirtQueueLayout::new(size);
-        // Allocate contiguous pages.
-        let dma = Dma::new(layout.size / PAGE_SIZE)?;
+
+        let layout = if transport.requires_legacy_layout() {
+            VirtQueueLayout::allocate_legacy(size)?
+        } else {
+            VirtQueueLayout::allocate_flexible(size)?
+        };
 
         transport.queue_set(
             idx,
             size as u32,
-            dma.paddr(),
-            dma.paddr() + layout.avail_offset,
-            dma.paddr() + layout.used_offset,
+            layout.descriptors_paddr(),
+            layout.driver_area_paddr(),
+            layout.device_area_paddr(),
         );
 
         let desc = NonNull::new(ptr::slice_from_raw_parts_mut(
-            dma.vaddr() as *mut Descriptor,
+            layout.descriptors_vaddr() as *mut Descriptor,
             size as usize,
         ))
         .unwrap();
-        let avail = NonNull::new((dma.vaddr() + layout.avail_offset) as *mut AvailRing).unwrap();
-        let used = NonNull::new((dma.vaddr() + layout.used_offset) as *mut UsedRing).unwrap();
+        let avail = NonNull::new(layout.avail_vaddr() as *mut AvailRing).unwrap();
+        let used = NonNull::new(layout.used_vaddr() as *mut UsedRing).unwrap();
 
         // Link descriptors together.
         for i in 0..(size - 1) {
@@ -79,7 +80,7 @@ impl<H: Hal> VirtQueue<H> {
         }
 
         Ok(VirtQueue {
-            dma,
+            layout,
             desc,
             avail,
             used,
@@ -288,31 +289,151 @@ impl<H: Hal> VirtQueue<H> {
 
 /// The inner layout of a VirtQueue.
 ///
-/// Ref: 2.6.2 Legacy Interfaces: A Note on Virtqueue Layout
-struct VirtQueueLayout {
-    avail_offset: usize,
-    used_offset: usize,
-    size: usize,
+/// Ref: 2.6 Split Virtqueues
+#[derive(Debug)]
+enum VirtQueueLayout<H: Hal> {
+    Legacy {
+        dma: Dma<H>,
+        avail_offset: usize,
+        used_offset: usize,
+    },
+    Modern {
+        /// The region used for the descriptor area and driver area.
+        driver_to_device_dma: Dma<H>,
+        /// The region used for the device area.
+        device_to_driver_dma: Dma<H>,
+        /// The offset from the start of the `driver_to_device_dma` region to the driver area
+        /// (available ring).
+        avail_offset: usize,
+    },
 }
 
-impl VirtQueueLayout {
-    fn new(queue_size: u16) -> Self {
-        assert!(
-            queue_size.is_power_of_two(),
-            "queue size should be a power of 2"
-        );
-        let queue_size = queue_size as usize;
-        let desc = size_of::<Descriptor>() * queue_size;
-        let avail = size_of::<u16>() * (3 + queue_size);
-        let used = size_of::<u16>() * 3 + size_of::<UsedElem>() * queue_size;
-        VirtQueueLayout {
+impl<H: Hal> VirtQueueLayout<H> {
+    /// Allocates a single DMA region containing all parts of the virtqueue, following the layout
+    /// required by legacy interfaces.
+    ///
+    /// Ref: 2.6.2 Legacy Interfaces: A Note on Virtqueue Layout
+    fn allocate_legacy(queue_size: u16) -> Result<Self> {
+        let (desc, avail, used) = queue_part_sizes(queue_size);
+        let size = align_up(desc + avail) + align_up(used);
+        // Allocate contiguous pages.
+        let dma = Dma::new(size / PAGE_SIZE, BufferDirection::Both)?;
+        Ok(Self::Legacy {
+            dma,
             avail_offset: desc,
             used_offset: align_up(desc + avail),
-            size: align_up(desc + avail) + align_up(used),
+        })
+    }
+
+    /// Allocates separate DMA regions for the the different parts of the virtqueue, as supported by
+    /// non-legacy interfaces.
+    ///
+    /// This is preferred over `allocate_legacy` where possible as it reduces memory fragmentation
+    /// and allows the HAL to know which DMA regions are used in which direction.
+    fn allocate_flexible(queue_size: u16) -> Result<Self> {
+        let (desc, avail, used) = queue_part_sizes(queue_size);
+        let driver_to_device_dma = Dma::new(pages(desc + avail), BufferDirection::DriverToDevice)?;
+        let device_to_driver_dma = Dma::new(pages(used), BufferDirection::DeviceToDriver)?;
+        Ok(Self::Modern {
+            driver_to_device_dma,
+            device_to_driver_dma,
+            avail_offset: desc,
+        })
+    }
+
+    /// Returns the physical address of the descriptor area.
+    fn descriptors_paddr(&self) -> PhysAddr {
+        match self {
+            Self::Legacy { dma, .. } => dma.paddr(),
+            Self::Modern {
+                driver_to_device_dma,
+                ..
+            } => driver_to_device_dma.paddr(),
+        }
+    }
+
+    /// Returns the virtual address of the descriptor table (in the descriptor area).
+    fn descriptors_vaddr(&self) -> VirtAddr {
+        match self {
+            Self::Legacy { dma, .. } => dma.vaddr(),
+            Self::Modern {
+                driver_to_device_dma,
+                ..
+            } => driver_to_device_dma.vaddr(),
+        }
+    }
+
+    /// Returns the physical address of the driver area.
+    fn driver_area_paddr(&self) -> PhysAddr {
+        match self {
+            Self::Legacy {
+                dma, avail_offset, ..
+            } => dma.paddr() + avail_offset,
+            Self::Modern {
+                driver_to_device_dma,
+                avail_offset,
+                ..
+            } => driver_to_device_dma.paddr() + avail_offset,
+        }
+    }
+
+    /// Returns the virtual address of the available ring (in the driver area).
+    fn avail_vaddr(&self) -> VirtAddr {
+        match self {
+            Self::Legacy {
+                dma, avail_offset, ..
+            } => dma.vaddr() + avail_offset,
+            Self::Modern {
+                driver_to_device_dma,
+                avail_offset,
+                ..
+            } => driver_to_device_dma.vaddr() + avail_offset,
+        }
+    }
+
+    /// Returns the physical address of the device area.
+    fn device_area_paddr(&self) -> PhysAddr {
+        match self {
+            Self::Legacy {
+                used_offset, dma, ..
+            } => dma.paddr() + used_offset,
+            Self::Modern {
+                device_to_driver_dma,
+                ..
+            } => device_to_driver_dma.paddr(),
+        }
+    }
+
+    /// Returns the virtual address of the used ring (in the driver area).
+    fn used_vaddr(&self) -> VirtAddr {
+        match self {
+            Self::Legacy {
+                dma, used_offset, ..
+            } => dma.vaddr() + used_offset,
+            Self::Modern {
+                device_to_driver_dma,
+                ..
+            } => device_to_driver_dma.vaddr(),
         }
     }
 }
 
+/// Returns the size in bytes of the descriptor table, available ring and used ring for a given
+/// queue size.
+///
+/// Ref: 2.6 Split Virtqueues
+fn queue_part_sizes(queue_size: u16) -> (usize, usize, usize) {
+    assert!(
+        queue_size.is_power_of_two(),
+        "queue size should be a power of 2"
+    );
+    let queue_size = queue_size as usize;
+    let desc = size_of::<Descriptor>() * queue_size;
+    let avail = size_of::<u16>() * (3 + queue_size);
+    let used = size_of::<u16>() * 3 + size_of::<UsedElem>() * queue_size;
+    (desc, avail, used)
+}
+
 #[repr(C, align(16))]
 #[derive(Debug)]
 pub(crate) struct Descriptor {
@@ -340,6 +461,9 @@ impl Descriptor {
             | match direction {
                 BufferDirection::DeviceToDriver => DescFlags::WRITE,
                 BufferDirection::DriverToDevice => DescFlags::empty(),
+                BufferDirection::Both => {
+                    panic!("Buffer passed to device should never use BufferDirection::Both.")
+                }
             };
     }
 

src/transport/fake.rs

@@ -46,6 +46,10 @@ impl<C> Transport for FakeTransport<C> {
         self.state.lock().unwrap().guest_page_size = guest_page_size;
     }
 
+    fn requires_legacy_layout(&self) -> bool {
+        false
+    }
+
     fn queue_set(
         &mut self,
         queue: u16,

src/transport/mmio.rs

@@ -371,6 +371,13 @@ impl Transport for MmioTransport {
         }
     }
 
+    fn requires_legacy_layout(&self) -> bool {
+        match self.version {
+            MmioVersion::Legacy => true,
+            MmioVersion::Modern => false,
+        }
+    }
+
     fn queue_set(
         &mut self,
         queue: u16,

src/transport/mod.rs

@@ -32,6 +32,11 @@ pub trait Transport {
     /// Sets the guest page size.
     fn set_guest_page_size(&mut self, guest_page_size: u32);
 
+    /// Returns whether the transport requires queues to use the legacy layout.
+    ///
+    /// Ref: 2.6.2 Legacy Interfaces: A Note on Virtqueue Layout
+    fn requires_legacy_layout(&self) -> bool;
+
     /// Sets up the given queue.
     fn queue_set(
         &mut self,

src/transport/pci.rs

@@ -262,6 +262,10 @@ impl Transport for PciTransport {
         // No-op, the PCI transport doesn't care.
     }
 
+    fn requires_legacy_layout(&self) -> bool {
+        false
+    }
+
     fn queue_set(
         &mut self,
         queue: u16,