From 7b2f4dda90c03b5f6ee0e75ad3e4b63cae013f6f Mon Sep 17 00:00:00 2001 From: 4lDO2 <4lDO2@protonmail.com> Date: Fri, 14 Jun 2024 08:18:45 +0200 Subject: [PATCH] Use scatter-gather list for virtio framebuffer. The current frame allocator limits requests to powers of two, between 4 KiB and 4 MiB. As such, a 8-bit color 1920x1080 framebuffer needs at least two allocations. --- common/src/dma.rs | 14 +++-- common/src/lib.rs | 1 + common/src/sgl.rs | 89 ++++++++++++++++++++++++++++++ graphics/virtio-gpud/src/scheme.rs | 76 ++++++++++++------------- 4 files changed, 134 insertions(+), 46 deletions(-) create mode 100644 common/src/sgl.rs diff --git a/common/src/dma.rs b/common/src/dma.rs index 1adbbc6392..4c723a001b 100644 --- a/common/src/dma.rs +++ b/common/src/dma.rs @@ -20,14 +20,18 @@ const DMA_MEMTY: MemoryType = { } }; +pub(crate) fn phys_contiguous_fd() -> Result { + Fd::open( + &format!("memory:zeroed@{DMA_MEMTY}?phys_contiguous"), + flag::O_CLOEXEC, + 0, + ) +} + fn alloc_and_map(length: usize) -> Result<(usize, *mut ())> { assert_eq!(length % PAGE_SIZE, 0); unsafe { - let fd = Fd::open( - &format!("memory:zeroed@{DMA_MEMTY}?phys_contiguous"), - flag::O_CLOEXEC, - 0, - )?; + let fd = phys_contiguous_fd()?; let virt = libredox::call::mmap(MmapArgs { fd: fd.raw(), offset: 0, // ignored diff --git a/common/src/lib.rs b/common/src/lib.rs index 2107a08660..b22d93bcda 100644 --- a/common/src/lib.rs +++ b/common/src/lib.rs @@ -6,6 +6,7 @@ use libredox::flag::{self, O_CLOEXEC, O_RDONLY, O_RDWR, O_WRONLY}; use syscall::PAGE_SIZE; pub mod dma; +pub mod sgl; #[derive(Clone, Copy, Debug)] pub enum MemoryType { diff --git a/common/src/sgl.rs b/common/src/sgl.rs new file mode 100644 index 0000000000..236ab85a05 --- /dev/null +++ b/common/src/sgl.rs @@ -0,0 +1,89 @@ +use std::num::NonZeroUsize; + +use libredox::call::MmapArgs; +use libredox::errno::EINVAL; +use libredox::error::{Error, Result}; +use libredox::flag::{MAP_PRIVATE, PROT_READ, PROT_WRITE}; +use syscall::{MAP_FIXED, PAGE_SIZE}; + +use crate::dma::phys_contiguous_fd; + +#[derive(Debug)] +pub struct Sgl { + virt: *mut u8, + unaligned_length: NonZeroUsize, + chunks: Vec, +} +#[derive(Debug)] +pub struct Chunk { + pub offset: usize, + pub phys: usize, + pub virt: *mut u8, + pub length: usize, +} + +impl Sgl { + pub fn new(unaligned_length: usize) -> Result { + let unaligned_length = NonZeroUsize::new(unaligned_length).ok_or(Error::new(EINVAL))?; + + unsafe { + let virt = libredox::call::mmap(MmapArgs { + flags: MAP_PRIVATE, + prot: PROT_READ | PROT_WRITE, + length: unaligned_length.get(), + + offset: 0, + fd: !0, + addr: core::ptr::null_mut(), + })?.cast::(); + + let mut this = Self { + virt, + unaligned_length, + chunks: Vec::new(), + }; + + let phys_contiguous_fd = phys_contiguous_fd()?; + + // TODO: Both PAGE_SIZE and MAX_ALLOC_SIZE should be dynamic. + let aligned_length = unaligned_length.get().next_multiple_of(PAGE_SIZE); + const MAX_ALLOC_SIZE: usize = 1 << 22; + + let mut offset = 0; + while offset < unaligned_length.get() { + let chunk_length = (aligned_length - offset).min(MAX_ALLOC_SIZE).next_power_of_two(); + libredox::call::mmap(MmapArgs { + addr: virt.add(offset).cast(), + flags: MAP_PRIVATE | (MAP_FIXED.bits() as u32), + prot: PROT_READ | PROT_WRITE, + length: chunk_length, + fd: phys_contiguous_fd.raw(), + + offset: 0, + })?; + let phys = syscall::virttophys(virt as usize + offset)?; + this.chunks.push(Chunk { offset, phys, length: (unaligned_length.get() - offset).min(chunk_length), virt: virt.add(offset) }); + offset += chunk_length; + } + + Ok(this) + } + } + pub fn chunks(&self) -> &[Chunk] { + &self.chunks + } + pub fn as_ptr(&self) -> *mut u8 { + self.virt + } + pub fn len(&self) -> usize { + self.unaligned_length.get() + } +} + +impl Drop for Sgl { + fn drop(&mut self) { + unsafe { + let _ = libredox::call::munmap(self.virt.cast(), self.unaligned_length.get()); + } + } +} diff --git a/graphics/virtio-gpud/src/scheme.rs b/graphics/virtio-gpud/src/scheme.rs index 907ebc873e..4782dfca59 100644 --- a/graphics/virtio-gpud/src/scheme.rs +++ b/graphics/virtio-gpud/src/scheme.rs @@ -1,12 +1,13 @@ +use std::cell::OnceCell; use std::collections::BTreeMap; use std::sync::atomic::{AtomicBool, AtomicU32, AtomicUsize, Ordering}; use std::sync::Arc; use inputd::Damage; -use common::dma::Dma; +use common::{dma::Dma, sgl}; -use syscall::{Error as SysError, SchemeMut, EAGAIN, EINVAL, MapFlags, PAGE_SIZE, KSMSG_MMAP, KSMSG_MMAP_PREP, KSMSG_MSYNC, KSMSG_MUNMAP}; +use syscall::{Error as SysError, MapFlags, SchemeMut, EAGAIN, EINVAL, PAGE_SIZE}; use virtio_core::spec::{Buffer, ChainBuilder, DescriptorFlags}; use virtio_core::transport::{Error, Queue, Transport}; @@ -32,9 +33,7 @@ pub struct Display<'a> { cursor_queue: Arc>, transport: Arc, - // TODO(andypython): Remove the need for the spin crate after the `once_cell` - // API is stabilized. - mapped: spin::Once, + mapped: OnceCell, width: u32, height: u32, @@ -56,7 +55,7 @@ impl<'a> Display<'a> { control_queue, cursor_queue, - mapped: spin::Once::new(), + mapped: OnceCell::new(), width: 1920, height: 1080, @@ -111,46 +110,41 @@ impl<'a> Display<'a> { Ok(()) } - async fn remap_screen(&self) -> Result { + // TODO: Is this a no-op? + async fn remap_screen(&self) -> Result<*mut u8, Error> { let bpp = 32; - let fb_size = (self.width as usize * self.height as usize * bpp / 8) - .next_multiple_of(syscall::PAGE_SIZE); - let mapped = *self.mapped.get().unwrap(); - let address = (unsafe { syscall::virttophys(mapped) }).map_err(libredox::error::Error::from)?; + let fb_size = self.width as usize * self.height as usize * bpp / 8; - self.map_screen_with(0, address, fb_size, mapped).await + let mapped = self.mapped.get().unwrap(); + self.map_screen_with(0, fb_size, mapped.as_ptr(), mapped.chunks()).await } - async fn map_screen(&self, offset: usize) -> Result { + async fn map_screen(&self, offset: usize) -> Result<*mut u8, Error> { if let Some(mapped) = self.mapped.get() { - return Ok(mapped + offset); + return Ok(mapped.as_ptr().wrapping_add(offset)); } let bpp = 32; - let fb_size = (self.width as usize * self.height as usize * bpp / 8) - .next_multiple_of(syscall::PAGE_SIZE); - let mut mapped = unsafe { Dma::zeroed_slice(fb_size)?.assume_init() }; + let fb_size = self.width as usize * self.height as usize * bpp / 8; + let mapped = sgl::Sgl::new(fb_size)?; unsafe { - core::ptr::write_bytes(mapped.as_mut_ptr() as *mut u8, 255, fb_size); + core::ptr::write_bytes(mapped.as_ptr() as *mut u8, 255, fb_size); } + let _ = self.mapped.set(mapped); + let mapped = self.mapped.get().unwrap(); - let virt = mapped.as_mut_ptr() as usize; - let phys = mapped.physical(); - core::mem::forget(mapped); - // TODO: Keep Dma - - self.map_screen_with(offset, phys, fb_size, virt).await + self.map_screen_with(offset, fb_size, mapped.as_ptr(), mapped.chunks()).await } async fn map_screen_with( &self, offset: usize, - address: usize, - size: usize, - mapped: usize, - ) -> Result { + _size: usize, + virt: *mut u8, + chunks: &[sgl::Chunk], + ) -> Result<*mut u8, Error> { // Create a host resource using `VIRTIO_GPU_CMD_RESOURCE_CREATE_2D`. let mut request = Dma::new(ResourceCreate2d::default())?; @@ -163,20 +157,21 @@ impl<'a> Display<'a> { // Use the allocated framebuffer from tthe guest ram, and attach it as backing // storage to the resource just created, using `VIRTIO_GPU_CMD_RESOURCE_ATTACH_BACKING`. - // - // TODO(andypython): Scatter lists are supported, so the framebuffer doesn’t need to be - // contignous in guest physical memory. - let entry = Dma::new(MemEntry { - address: address as u64, - length: size as u32, - padding: 0, - })?; - let attach_request = Dma::new(AttachBacking::new(self.resource_id, 1))?; + let mut mem_entries = unsafe { Dma::zeroed_slice(chunks.len())?.assume_init() }; + for (entry, chunk) in mem_entries.iter_mut().zip(chunks.iter()) { + *entry = MemEntry { + address: chunk.phys as u64, + length: chunk.length.next_multiple_of(PAGE_SIZE) as u32, + padding: 0, + }; + } + + let attach_request = Dma::new(AttachBacking::new(self.resource_id, mem_entries.len() as u32))?; let header = Dma::new(ControlHeader::default())?; let command = ChainBuilder::new() .chain(Buffer::new(&attach_request)) - .chain(Buffer::new(&entry)) + .chain(Buffer::new_unsized(&mem_entries)) .chain(Buffer::new(&header).flags(DescriptorFlags::WRITE_ONLY)) .build(); @@ -192,9 +187,8 @@ impl<'a> Display<'a> { assert_eq!(header.ty.get(), CommandTy::RespOkNodata); self.flush(None).await?; - self.mapped.call_once(|| mapped); - Ok(mapped + offset) + Ok(virt.wrapping_add(offset)) } /// If `damage` is `None`, the entire screen is flushed. @@ -473,7 +467,7 @@ impl<'a> SchemeMut for Scheme<'a> { log::info!("KSMSG MMAP {} {:?} {} {}", id, flags, offset, size); match self.handles.get(&id).ok_or(SysError::new(EINVAL))? { Handle::Vt { display, .. } => { - Ok(futures::executor::block_on(display.map_screen(offset as usize)).unwrap()) + Ok(futures::executor::block_on(display.map_screen(offset as usize)).unwrap() as usize) } _ => unreachable!(), }