From 2344206c21d2e730e9dc239d01c44ceb5c780a2a Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 26 Dec 2024 17:36:40 +0100 Subject: [PATCH 1/9] graphics/virtio-gpud: Bundle resource id and resource mapping --- graphics/virtio-gpud/src/scheme.rs | 64 +++++++++++------------------- 1 file changed, 24 insertions(+), 40 deletions(-) diff --git a/graphics/virtio-gpud/src/scheme.rs b/graphics/virtio-gpud/src/scheme.rs index deb7439843..60c49b0f19 100644 --- a/graphics/virtio-gpud/src/scheme.rs +++ b/graphics/virtio-gpud/src/scheme.rs @@ -27,14 +27,18 @@ impl Into for Damage { } } +struct GpuResource { + id: ResourceId, + sgl: sgl::Sgl, +} + pub struct Display<'a> { control_queue: Arc>, cursor_queue: Arc>, transport: Arc, active_vt: RefCell, - vts_map: RefCell>, - vts_res: RefCell>, + vts: RefCell>, width: u32, height: u32, @@ -54,8 +58,7 @@ impl<'a> Display<'a> { cursor_queue, active_vt: RefCell::new(0), - vts_map: RefCell::new(HashMap::new()), - vts_res: RefCell::new(HashMap::new()), + vts: RefCell::new(HashMap::new()), width: 1920, height: 1080, @@ -91,34 +94,19 @@ impl<'a> Display<'a> { Ok(()) } - async fn mmap_screen(&self, vt: usize, offset: usize) -> Result<*mut u8, Error> { - if let Some(sgl) = self.vts_map.borrow().get(&vt) { - return Ok(sgl.as_ptr().wrapping_add(offset)); + async fn res_for_screen(&self, vt: usize) -> Result<(ResourceId, *mut u8), Error> { + if let Some(res) = self.vts.borrow().get(&vt) { + return Ok((res.id, res.sgl.as_ptr())); } let bpp = 32; let fb_size = self.width as usize * self.height as usize * bpp / 8; - let mapped = sgl::Sgl::new(fb_size)?; + let sgl = sgl::Sgl::new(fb_size)?; unsafe { - core::ptr::write_bytes(mapped.as_ptr() as *mut u8, 255, fb_size); + core::ptr::write_bytes(sgl.as_ptr() as *mut u8, 255, fb_size); } - let mut mapped_vts = self.vts_map.borrow_mut(); - let sgl = mapped_vts.entry(vt).or_insert(mapped); - Ok(sgl.as_ptr().wrapping_add(offset)) - } - - async fn create_res_for_screen(&self, vt: usize) -> Result { - if let Some(&res_id) = self.vts_res.borrow().get(&vt) { - return Ok(res_id); - } - - self.mmap_screen(vt, 0).await?; - - let vts_map = self.vts_map.borrow(); - let mapped = &vts_map.get(&vt).unwrap(); - let res_id = ResourceId::alloc(); // Create a host resource using `VIRTIO_GPU_CMD_RESOURCE_CREATE_2D`. @@ -135,8 +123,8 @@ 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`. - let mut mem_entries = unsafe { Dma::zeroed_slice(mapped.chunks().len())?.assume_init() }; - for (entry, chunk) in mem_entries.iter_mut().zip(mapped.chunks().iter()) { + let mut mem_entries = unsafe { Dma::zeroed_slice(sgl.chunks().len())?.assume_init() }; + for (entry, chunk) in mem_entries.iter_mut().zip(sgl.chunks().iter()) { *entry = MemEntry { address: chunk.phys as u64, length: chunk.length.next_multiple_of(PAGE_SIZE) as u32, @@ -155,13 +143,15 @@ impl<'a> Display<'a> { self.control_queue.send(command).await; assert_eq!(header.ty.get(), CommandTy::RespOkNodata); - let mut mapped_vts = self.vts_res.borrow_mut(); - mapped_vts.insert(vt, res_id); - Ok(res_id) + let mut mapped_vts = self.vts.borrow_mut(); + let res = mapped_vts + .entry(vt) + .or_insert(GpuResource { id: res_id, sgl }); + Ok((res.id, res.sgl.as_ptr())) } async fn set_scanout(&self, vt: usize) -> Result<(), Error> { - let res_id = self.create_res_for_screen(vt).await?; + let (res_id, _) = self.res_for_screen(vt).await?; let scanout_request = Dma::new(SetScanout::new( self.id as u32, @@ -178,12 +168,7 @@ impl<'a> Display<'a> { /// If `damage` is `None`, the entire screen is flushed. async fn flush(&self, vt: usize, damage: Option<&[Damage]>) -> Result<(), Error> { - let Some(&res_id) = self.vts_res.borrow().get(&vt) else { - // The resource is not yet created. Ignore the damage. We will write the entire backing - // storage to the resource once we create the resource, which is equivalent to damaging - // the entire resource. - return Ok(()); - }; + let (res_id, _) = self.res_for_screen(vt).await?; let req = Dma::new(XferToHost2d::new( res_id, @@ -430,9 +415,8 @@ impl<'a> Scheme for GpuScheme<'a> { ) -> syscall::Result { log::info!("KSMSG MMAP {} {:?} {} {}", id, flags, offset, size); let handle = self.handles.get(&id).ok_or(SysError::new(EINVAL))?; - let ptr = - futures::executor::block_on(handle.display.mmap_screen(handle.vt, offset as usize)) - .unwrap(); - Ok(ptr as usize) + let (_, ptr) = + futures::executor::block_on(handle.display.res_for_screen(handle.vt)).unwrap(); + Ok(unsafe { ptr.offset(offset as isize) } as usize) } } From 62f1a80c3ee2a76ed3e75d691338ac3f79dc94d9 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 26 Dec 2024 17:53:45 +0100 Subject: [PATCH 2/9] graphics/virtio-gpud: Move resources from Display to GpuScheme Resources are global for the entire virtio-gpud device, not local to a single display. In the future resource creation will become entirely detached from specific displays. --- graphics/virtio-gpud/src/scheme.rs | 176 ++++++++++++++--------------- 1 file changed, 87 insertions(+), 89 deletions(-) diff --git a/graphics/virtio-gpud/src/scheme.rs b/graphics/virtio-gpud/src/scheme.rs index 60c49b0f19..b72f1aa5f6 100644 --- a/graphics/virtio-gpud/src/scheme.rs +++ b/graphics/virtio-gpud/src/scheme.rs @@ -1,4 +1,3 @@ -use std::cell::RefCell; use std::collections::{BTreeMap, HashMap}; use std::sync::atomic::{AtomicUsize, Ordering}; @@ -32,39 +31,18 @@ struct GpuResource { sgl: sgl::Sgl, } -pub struct Display<'a> { - control_queue: Arc>, - cursor_queue: Arc>, - transport: Arc, - - active_vt: RefCell, - vts: RefCell>, - +#[derive(Debug, Copy, Clone)] +pub struct Display { width: u32, height: u32, - - id: usize, } -impl<'a> Display<'a> { - pub fn new( - control_queue: Arc>, - cursor_queue: Arc>, - transport: Arc, - id: usize, - ) -> Self { +impl Display { + pub fn new() -> Self { Self { - control_queue, - cursor_queue, - - active_vt: RefCell::new(0), - vts: RefCell::new(HashMap::new()), - + // FIXME use the actual screen size width: 1920, height: 1080, - transport, - - id, } } @@ -75,7 +53,9 @@ impl<'a> Display<'a> { buffer[..path.len()].copy_from_slice(path.as_bytes()); Ok(path.len()) } +} +impl GpuScheme<'_> { async fn send_request(&self, request: Dma) -> Result, Error> { let header = Dma::new(ControlHeader::default())?; let command = ChainBuilder::new() @@ -94,13 +74,18 @@ impl<'a> Display<'a> { Ok(()) } - async fn res_for_screen(&self, vt: usize) -> Result<(ResourceId, *mut u8), Error> { - if let Some(res) = self.vts.borrow().get(&vt) { + async fn res_for_screen( + &mut self, + vt: usize, + screen: usize, + ) -> Result<(ResourceId, *mut u8), Error> { + if let Some(res) = self.vts.entry(vt).or_default().get(&screen) { return Ok((res.id, res.sgl.as_ptr())); } let bpp = 32; - let fb_size = self.width as usize * self.height as usize * bpp / 8; + let fb_size = + self.displays[screen].width as usize * self.displays[screen].height as usize * bpp / 8; let sgl = sgl::Sgl::new(fb_size)?; unsafe { @@ -112,8 +97,8 @@ impl<'a> Display<'a> { // Create a host resource using `VIRTIO_GPU_CMD_RESOURCE_CREATE_2D`. let mut request = Dma::new(ResourceCreate2d::default())?; - request.set_width(self.width); - request.set_height(self.height); + request.set_width(self.displays[screen].width); + request.set_height(self.displays[screen].height); request.set_format(ResourceFormat::Bgrx); request.set_resource_id(res_id); @@ -143,40 +128,52 @@ impl<'a> Display<'a> { self.control_queue.send(command).await; assert_eq!(header.ty.get(), CommandTy::RespOkNodata); - let mut mapped_vts = self.vts.borrow_mut(); - let res = mapped_vts + let res = self + .vts .entry(vt) + .or_default() + .entry(screen) .or_insert(GpuResource { id: res_id, sgl }); Ok((res.id, res.sgl.as_ptr())) } - async fn set_scanout(&self, vt: usize) -> Result<(), Error> { - let (res_id, _) = self.res_for_screen(vt).await?; + async fn set_scanout(&mut self, vt: usize, screen: usize) -> Result<(), Error> { + let (res_id, _) = self.res_for_screen(vt, screen).await?; let scanout_request = Dma::new(SetScanout::new( - self.id as u32, + screen as u32, res_id, - GpuRect::new(0, 0, self.width, self.height), + GpuRect::new( + 0, + 0, + self.displays[screen].width, + self.displays[screen].height, + ), ))?; let header = self.send_request(scanout_request).await?; assert_eq!(header.ty.get(), CommandTy::RespOkNodata); - self.flush(vt, None).await?; + self.flush(vt, screen, None).await?; Ok(()) } /// If `damage` is `None`, the entire screen is flushed. - async fn flush(&self, vt: usize, damage: Option<&[Damage]>) -> Result<(), Error> { - let (res_id, _) = self.res_for_screen(vt).await?; + async fn flush( + &mut self, + vt: usize, + screen: usize, + damage: Option<&[Damage]>, + ) -> Result<(), Error> { + let (res_id, _) = self.res_for_screen(vt, screen).await?; let req = Dma::new(XferToHost2d::new( res_id, GpuRect { x: 0, y: 0, - width: self.width, - height: self.height, + width: self.displays[screen].width, + height: self.displays[screen].height, }, 0, ))?; @@ -187,7 +184,12 @@ impl<'a> Display<'a> { for damage in damage { self.flush_resource(ResourceFlush::new( res_id, - damage.clip(self.width as i32, self.height as i32).into(), + damage + .clip( + self.displays[screen].width as i32, + self.displays[screen].height as i32, + ) + .into(), )) .await?; } @@ -197,8 +199,8 @@ impl<'a> Display<'a> { GpuRect { x: 0, y: 0, - width: self.width, - height: self.height, + width: self.displays[screen].width, + height: self.displays[screen].height, }, )) .await?; @@ -207,17 +209,24 @@ impl<'a> Display<'a> { } } -struct Handle<'a> { - display: Arc>, +struct Handle { + screen: usize, vt: usize, } pub struct GpuScheme<'a> { - handles: BTreeMap>, + control_queue: Arc>, + cursor_queue: Arc>, + transport: Arc, + pub inputd_handle: inputd::DisplayHandle, + + handles: BTreeMap, /// Counter used for file descriptor allocation. next_id: AtomicUsize, - displays: Vec>>, - pub inputd_handle: inputd::DisplayHandle, + + active_vt: usize, + vts: HashMap>, + displays: Vec, } impl<'a> GpuScheme<'a> { @@ -227,50 +236,42 @@ impl<'a> GpuScheme<'a> { cursor_queue: Arc>, transport: Arc, ) -> Result, Error> { - let displays = Self::probe( - control_queue.clone(), - cursor_queue.clone(), - transport.clone(), - config, - ) - .await?; + let displays = Self::probe(control_queue.clone(), config).await?; let inputd_handle = inputd::DisplayHandle::new("virtio-gpu").unwrap(); Ok(Self { + control_queue, + cursor_queue, + transport, + inputd_handle, + handles: BTreeMap::new(), next_id: AtomicUsize::new(0), + + active_vt: 0, + vts: HashMap::new(), displays, - inputd_handle, }) } async fn probe( control_queue: Arc>, - cursor_queue: Arc>, - transport: Arc, config: &GpuConfig, - ) -> Result>>, Error> { + ) -> Result, Error> { let mut display_info = Self::get_display_info(control_queue.clone()).await?; let displays = &mut display_info.display_info[..config.num_scanouts() as usize]; let mut result = vec![]; - for (id, info) in displays.iter().enumerate() { + for info in displays.iter() { log::info!( "virtio-gpu: opening display ({}x{}px)", info.rect().width, info.rect().height ); - let display = Display::new( - control_queue.clone(), - cursor_queue.clone(), - transport.clone(), - id, - ); - - result.push(Arc::new(display)); + result.push(Display::new()); } Ok(result) @@ -299,13 +300,13 @@ impl<'a> GpuScheme<'a> { VtEventKind::Activate => { log::info!("activate {}", vt_event.vt); - for display in &self.displays { + for id in 0..self.displays.len() { log::warn!("virtio-gpu: activating"); - futures::executor::block_on(display.set_scanout(vt_event.vt)).unwrap(); - - *display.active_vt.borrow_mut() = vt_event.vt; + futures::executor::block_on(self.set_scanout(vt_event.vt, id)).unwrap(); } + + self.active_vt = vt_event.vt; } VtEventKind::Deactivate => { @@ -334,33 +335,30 @@ impl<'a> Scheme for GpuScheme<'a> { dbg!(vt, id); - let display = self.displays.get(id).ok_or(SysError::new(EINVAL))?; + if id >= self.displays.len() { + return Err(SysError::new(EINVAL)); + }; let fd = self.next_id.fetch_add(1, Ordering::SeqCst); - self.handles.insert( - fd, - Handle { - display: display.clone(), - vt, - }, - ); + self.handles.insert(fd, Handle { screen: id, vt }); Ok(fd) } fn fpath(&mut self, id: usize, buf: &mut [u8]) -> syscall::Result { let handle = self.handles.get(&id).unwrap(); - let bytes_copied = futures::executor::block_on(handle.display.get_fpath(buf)).unwrap(); + let bytes_copied = + futures::executor::block_on(self.displays[handle.screen].get_fpath(buf)).unwrap(); Ok(bytes_copied) } fn fsync(&mut self, id: usize) -> syscall::Result { let handle = self.handles.get(&id).ok_or(SysError::new(EINVAL))?; - if handle.vt != *handle.display.active_vt.borrow() { + if handle.vt != self.active_vt { // This is a protection against background VT's spamming us with flush requests. We will // flush the resource on the next scanout anyway return Ok(0); } - futures::executor::block_on(handle.display.flush(handle.vt, None)).unwrap(); + futures::executor::block_on(self.flush(handle.vt, handle.screen, None)).unwrap(); Ok(0) } @@ -385,7 +383,7 @@ impl<'a> Scheme for GpuScheme<'a> { ) -> syscall::Result { let handle = self.handles.get(&id).ok_or(SysError::new(EINVAL))?; - if handle.vt != *handle.display.active_vt.borrow() { + if handle.vt != self.active_vt { // This is a protection against background VT's spamming us with flush requests. We will // flush the resource on the next scanout anyway return Ok(buf.len()); @@ -398,7 +396,7 @@ impl<'a> Scheme for GpuScheme<'a> { ) }; - futures::executor::block_on(handle.display.flush(handle.vt, Some(damage))).unwrap(); + futures::executor::block_on(self.flush(handle.vt, handle.screen, Some(damage))).unwrap(); Ok(buf.len()) } @@ -416,7 +414,7 @@ impl<'a> Scheme for GpuScheme<'a> { log::info!("KSMSG MMAP {} {:?} {} {}", id, flags, offset, size); let handle = self.handles.get(&id).ok_or(SysError::new(EINVAL))?; let (_, ptr) = - futures::executor::block_on(handle.display.res_for_screen(handle.vt)).unwrap(); + futures::executor::block_on(self.res_for_screen(handle.vt, handle.screen)).unwrap(); Ok(unsafe { ptr.offset(offset as isize) } as usize) } } From 09de72e9a850a8664204f5ea0697d0c8e6449f87 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 26 Dec 2024 20:50:44 +0100 Subject: [PATCH 3/9] common: Fix mmap overwriting unintended memory in Sgl If unaligned_length is for example 8193, the initial reservation would be rounded up to the next page resulting in 12288. However the physmap would previously round up to the next power of two forming 16384, potentially overwriting one page of data directly after the reservation. In case of bigger allocations, more pages could be overwritten. --- common/src/sgl.rs | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/common/src/sgl.rs b/common/src/sgl.rs index ba26d3f00b..9bab7ad7c4 100644 --- a/common/src/sgl.rs +++ b/common/src/sgl.rs @@ -15,6 +15,8 @@ use crate::dma::phys_contiguous_fd; pub struct Sgl { /// A raw pointer to the SGL in virtual memory virt: *mut u8, + /// The length of the allocated memory, guaranteed to be a multiple of [PAGE_SIZE]. + aligned_length: usize, /// The length of the allocated memory. This value is NOT guaranteed to be a multiple of [PAGE_SIZE] unaligned_length: NonZeroUsize, /// The vector of chunks tracked by this [Sgl] object. This is the sparsely-populated vector in the SGL algorithm. @@ -39,15 +41,20 @@ impl Sgl { /// /// # Arguments /// - /// 'unaligned_length: [usize]' - The length of the SGL, not aligned to the nearest page. + /// 'unaligned_length: [usize]' - The length of the SGL, not necessarily aligned to the nearest + /// page. pub fn new(unaligned_length: usize) -> Result { let unaligned_length = NonZeroUsize::new(unaligned_length).ok_or(Error::new(EINVAL))?; + // 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; + unsafe { let virt = libredox::call::mmap(MmapArgs { flags: MAP_PRIVATE, prot: PROT_READ | PROT_WRITE, - length: unaligned_length.get(), + length: aligned_length, offset: 0, fd: !0, @@ -57,21 +64,23 @@ impl Sgl { let mut this = Self { virt, + aligned_length, 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) + while offset < aligned_length { + let preferred_chunk_length = (aligned_length - offset) .min(MAX_ALLOC_SIZE) .next_power_of_two(); + let chunk_length = if preferred_chunk_length > aligned_length - offset { + preferred_chunk_length / 2 + } else { + preferred_chunk_length + }; libredox::call::mmap(MmapArgs { addr: virt.add(offset).cast(), flags: MAP_PRIVATE | (MAP_FIXED.bits() as u32), @@ -112,7 +121,7 @@ impl Sgl { impl Drop for Sgl { fn drop(&mut self) { unsafe { - let _ = libredox::call::munmap(self.virt.cast(), self.unaligned_length.get()); + let _ = libredox::call::munmap(self.virt.cast(), self.aligned_length); } } } From 3ed3ff2edbbbe385bbd98b2633fdae7cb837b9ff Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 26 Dec 2024 20:57:53 +0100 Subject: [PATCH 4/9] graphics/fbcond: Handle framebuffer resize during handoff --- graphics/fbcond/src/display.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/graphics/fbcond/src/display.rs b/graphics/fbcond/src/display.rs index 46785796df..053b585563 100644 --- a/graphics/fbcond/src/display.rs +++ b/graphics/fbcond/src/display.rs @@ -61,11 +61,13 @@ impl Display { let (mut new_display_file, width, height) = Self::open_display(&self.input_handle).unwrap(); - eprintln!("fbcond: Opened new display"); + eprintln!("fbcond: Opened new display with size {width}x{height}"); match display_fd_map(width, height, &mut new_display_file) { Ok(offscreen) => { self.offscreen = offscreen; + self.width = width; + self.height = height; self.display_file = new_display_file; eprintln!("fbcond: Mapped new display"); @@ -108,6 +110,8 @@ impl Display { match display_fd_map(width, height, &mut self.display_file) { Ok(offscreen) => { self.offscreen = offscreen; + self.width = width; + self.height = height; } Err(err) => { eprintln!("failed to resize display to {}x{}: {}", width, height, err); From 1b6feeb7c7f0da84ea22c7fa3168efa8653a9006 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 26 Dec 2024 20:59:08 +0100 Subject: [PATCH 5/9] graphics/virtio-gpud: Use VM window size as display size Rather than hard coding 1920x1080. This doesn't yet handle resizing the VM window at runtime though. --- graphics/virtio-gpud/src/scheme.rs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/graphics/virtio-gpud/src/scheme.rs b/graphics/virtio-gpud/src/scheme.rs index b72f1aa5f6..e9c1bef067 100644 --- a/graphics/virtio-gpud/src/scheme.rs +++ b/graphics/virtio-gpud/src/scheme.rs @@ -38,14 +38,6 @@ pub struct Display { } impl Display { - pub fn new() -> Self { - Self { - // FIXME use the actual screen size - width: 1920, - height: 1080, - } - } - async fn get_fpath(&self, buffer: &mut [u8]) -> Result { let path = format!("display.virtio-gpu:3.0/{}/{}", self.width, self.height); @@ -271,7 +263,10 @@ impl<'a> GpuScheme<'a> { info.rect().height ); - result.push(Display::new()); + result.push(Display { + width: info.rect().width, + height: info.rect().height, + }); } Ok(result) From 3fbf0fecbb4daa3f31b06de5b361a83f1d5e1c9c Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 26 Dec 2024 21:57:41 +0100 Subject: [PATCH 6/9] graphics/console-draw: Fix resizing to a smaller display Without this commit ransid would generate move events with an underflowed height which take forever to process. --- graphics/console-draw/src/lib.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/graphics/console-draw/src/lib.rs b/graphics/console-draw/src/lib.rs index d52e46c57f..9c94ee934c 100644 --- a/graphics/console-draw/src/lib.rs +++ b/graphics/console-draw/src/lib.rs @@ -124,9 +124,13 @@ impl TextScreen { buf: &[u8], input: &mut VecDeque, ) -> Vec { - self.console.state.w = map.width / 8; - self.console.state.h = map.height / 16; - self.console.state.bottom_margin = (map.height / 16).saturating_sub(1); + self.console.resize(map.width / 8, map.height / 16); + if self.console.state.x > self.console.state.w { + self.console.state.x = self.console.state.w; + } + if self.console.state.y > self.console.state.h { + self.console.state.y = self.console.state.h; + } if self.console.state.cursor && self.console.state.x < self.console.state.w From bf6e16c8c9f14adbe25f112ec20b6af752cbb0b9 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 26 Dec 2024 21:58:07 +0100 Subject: [PATCH 7/9] graphics/console-draw: Slightly simplify TextScreen::write --- graphics/console-draw/src/lib.rs | 103 +++++++++++++++---------------- 1 file changed, 50 insertions(+), 53 deletions(-) diff --git a/graphics/console-draw/src/lib.rs b/graphics/console-draw/src/lib.rs index 9c94ee934c..1b4c386cf1 100644 --- a/graphics/console-draw/src/lib.rs +++ b/graphics/console-draw/src/lib.rs @@ -142,68 +142,65 @@ impl TextScreen { self.changed.insert(y); } - { - let changed = &mut self.changed; - self.console.write(buf, |event| match event { - ransid::Event::Char { - x, - y, - c, - color, - bold, - .. - } => { - Self::char(map, x * 8, y * 16, c, color.as_rgb(), bold, false); - changed.insert(y); + self.console.write(buf, |event| match event { + ransid::Event::Char { + x, + y, + c, + color, + bold, + .. + } => { + Self::char(map, x * 8, y * 16, c, color.as_rgb(), bold, false); + self.changed.insert(y); + } + ransid::Event::Input { data } => input.extend(data), + ransid::Event::Rect { x, y, w, h, color } => { + Self::rect(map, x * 8, y * 16, w * 8, h * 16, color.as_rgb()); + for y2 in y..y + h { + self.changed.insert(y2); } - ransid::Event::Input { data } => input.extend(data), - ransid::Event::Rect { x, y, w, h, color } => { - Self::rect(map, x * 8, y * 16, w * 8, h * 16, color.as_rgb()); - for y2 in y..y + h { - changed.insert(y2); - } - } - ransid::Event::ScreenBuffer { .. } => (), - ransid::Event::Move { - from_x, - from_y, - to_x, - to_y, - w, - h, - } => { - let width = map.width; - let pixels = unsafe { &mut *map.offscreen }; + } + ransid::Event::ScreenBuffer { .. } => (), + ransid::Event::Move { + from_x, + from_y, + to_x, + to_y, + w, + h, + } => { + let width = map.width; + let pixels = unsafe { &mut *map.offscreen }; - for raw_y in 0..h { - let y = if from_y > to_y { raw_y } else { h - raw_y - 1 }; + for raw_y in 0..h { + let y = if from_y > to_y { raw_y } else { h - raw_y - 1 }; - for pixel_y in 0..16 { - { - let off_from = ((from_y + y) * 16 + pixel_y) * width + from_x * 8; - let off_to = ((to_y + y) * 16 + pixel_y) * width + to_x * 8; - let len = w * 8; + for pixel_y in 0..16 { + { + let off_from = ((from_y + y) * 16 + pixel_y) * width + from_x * 8; + let off_to = ((to_y + y) * 16 + pixel_y) * width + to_x * 8; + let len = w * 8; - if off_from + len <= pixels.len() && off_to + len <= pixels.len() { - unsafe { - let data_ptr = pixels.as_mut_ptr() as *mut u32; - ptr::copy( - data_ptr.offset(off_from as isize), - data_ptr.offset(off_to as isize), - len, - ); - } + if off_from + len <= pixels.len() && off_to + len <= pixels.len() { + unsafe { + let data_ptr = pixels.as_mut_ptr() as *mut u32; + ptr::copy( + data_ptr.offset(off_from as isize), + data_ptr.offset(off_to as isize), + len, + ); } } } - - changed.insert(to_y + y); } + + self.changed.insert(to_y + y); } - ransid::Event::Resize { .. } => (), - ransid::Event::Title { .. } => (), - }); - } + } + ransid::Event::Resize { .. } => (), + ransid::Event::Title { .. } => (), + }); if self.console.state.cursor && self.console.state.x < self.console.state.w From 6f188cf7efa3a6460a7ad234002e2c23d8678dec Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 26 Dec 2024 21:58:41 +0100 Subject: [PATCH 8/9] graphics/fbcond: Unmap old offscreen buffer on handoff and resize --- graphics/fbcond/src/display.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/graphics/fbcond/src/display.rs b/graphics/fbcond/src/display.rs index 053b585563..cf2dbc528a 100644 --- a/graphics/fbcond/src/display.rs +++ b/graphics/fbcond/src/display.rs @@ -65,6 +65,10 @@ impl Display { match display_fd_map(width, height, &mut new_display_file) { Ok(offscreen) => { + unsafe { + display_fd_unmap(self.offscreen); + } + self.offscreen = offscreen; self.width = width; self.height = height; @@ -109,6 +113,10 @@ impl Display { pub fn resize(&mut self, width: usize, height: usize) { match display_fd_map(width, height, &mut self.display_file) { Ok(offscreen) => { + unsafe { + display_fd_unmap(self.offscreen); + } + self.offscreen = offscreen; self.width = width; self.height = height; From b7f4af9bd2bf958caaf9c66a327564d6832676a8 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 26 Dec 2024 16:11:48 +0100 Subject: [PATCH 9/9] graphics/virtio-gpud: Bring scheme impl closer in line with vesad --- graphics/virtio-gpud/src/scheme.rs | 35 ++++++++++++------------------ 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/graphics/virtio-gpud/src/scheme.rs b/graphics/virtio-gpud/src/scheme.rs index e9c1bef067..35933fad9d 100644 --- a/graphics/virtio-gpud/src/scheme.rs +++ b/graphics/virtio-gpud/src/scheme.rs @@ -7,7 +7,7 @@ use common::{dma::Dma, sgl}; use inputd::{Damage, VtEvent, VtEventKind}; use redox_scheme::Scheme; -use syscall::{Error as SysError, MapFlags, EINVAL, PAGE_SIZE}; +use syscall::{Error as SysError, MapFlags, EBADF, EINVAL, PAGE_SIZE}; use virtio_core::spec::{Buffer, ChainBuilder, DescriptorFlags}; use virtio_core::transport::{Error, Queue, Transport}; @@ -37,16 +37,6 @@ pub struct Display { height: u32, } -impl Display { - async fn get_fpath(&self, buffer: &mut [u8]) -> Result { - let path = format!("display.virtio-gpu:3.0/{}/{}", self.width, self.height); - - // Copy the path into the target buffer. - buffer[..path.len()].copy_from_slice(path.as_bytes()); - Ok(path.len()) - } -} - impl GpuScheme<'_> { async fn send_request(&self, request: Dma) -> Result, Error> { let header = Dma::new(ControlHeader::default())?; @@ -341,13 +331,16 @@ impl<'a> Scheme for GpuScheme<'a> { fn fpath(&mut self, id: usize, buf: &mut [u8]) -> syscall::Result { let handle = self.handles.get(&id).unwrap(); - let bytes_copied = - futures::executor::block_on(self.displays[handle.screen].get_fpath(buf)).unwrap(); - Ok(bytes_copied) + let path = format!( + "display.virtio-gpu:3.0/{}/{}", + self.displays[handle.screen].width, self.displays[handle.screen].height + ); + buf[..path.len()].copy_from_slice(path.as_bytes()); + Ok(path.len()) } fn fsync(&mut self, id: usize) -> syscall::Result { - let handle = self.handles.get(&id).ok_or(SysError::new(EINVAL))?; + let handle = self.handles.get(&id).ok_or(SysError::new(EBADF))?; if handle.vt != self.active_vt { // This is a protection against background VT's spamming us with flush requests. We will // flush the resource on the next scanout anyway @@ -359,14 +352,13 @@ impl<'a> Scheme for GpuScheme<'a> { fn read( &mut self, - _id: usize, + id: usize, _buf: &mut [u8], _offset: u64, _fcntl_flags: u32, ) -> syscall::Result { - // TODO: figure out how to get input lol - log::warn!("virtio_gpu::read is a stub!"); - Ok(0) + let _handle = self.handles.get(&id).ok_or(SysError::new(EBADF))?; + Err(SysError::new(EINVAL)) } fn write( @@ -376,7 +368,7 @@ impl<'a> Scheme for GpuScheme<'a> { _offset: u64, _fcntl_flags: u32, ) -> syscall::Result { - let handle = self.handles.get(&id).ok_or(SysError::new(EINVAL))?; + let handle = self.handles.get(&id).ok_or(SysError::new(EBADF))?; if handle.vt != self.active_vt { // This is a protection against background VT's spamming us with flush requests. We will @@ -396,7 +388,8 @@ impl<'a> Scheme for GpuScheme<'a> { Ok(buf.len()) } - fn close(&mut self, _id: usize) -> syscall::Result { + fn close(&mut self, id: usize) -> syscall::Result { + self.handles.remove(&id).ok_or(SysError::new(EBADF))?; Ok(0) } fn mmap_prep(