diff --git a/src/proto/console/gop.rs b/src/proto/console/gop.rs index 56d84eea0..f2c78ff72 100644 --- a/src/proto/console/gop.rs +++ b/src/proto/console/gop.rs @@ -23,6 +23,8 @@ //! In theory, a buffer with a width of 640 should have (640 * 4) bytes per row, //! but in practice there might be some extra padding used for efficiency. +use core::marker::PhantomData; +use core::mem; use core::ptr; use crate::{Completion, Result, Status}; @@ -249,29 +251,20 @@ impl GraphicsOutput { *self.mode.info } - /// Returns the base pointer and size (in bytes) of the framebuffer - /// - /// To use this pointer safely, a caller must... - /// - Honor the pixel format specificed by the mode info - /// - Keep pointer accesses in bound - /// - Use volatile writes so that the compiler does not optimize out or - /// aggressively reorder framebuffer accesses - /// - Make sure that the pointer is not used beyond its validity limit - /// - /// Although the UEFI spec makes no clear statement about framebuffer - /// pointer validity, it seems reasonable to expect the framebuffer pointer - /// to be valid until the next mode change. In the future, a safer interface - /// may be introduced, which would enforce volatile writes and automatically - /// check for dangling pointers using Rust's borrow checker. - pub fn frame_buffer(&mut self) -> (*mut u8, usize) { + /// Access the frame buffer directly + pub fn frame_buffer(&mut self) -> FrameBuffer { assert!( self.mode.info.format != PixelFormat::BltOnly, "Cannot access the framebuffer in a Blt-only mode" ); - let data = self.mode.fb_address as *mut u8; - let len = self.mode.fb_size; + let base = self.mode.fb_address as *mut u8; + let size = self.mode.fb_size; - (data, len) + FrameBuffer { + base, + size, + _lifetime: PhantomData, + } } } @@ -519,3 +512,88 @@ pub enum BltOp<'a> { dims: (usize, usize), }, } + +/// Direct access to a memory-mapped frame buffer +pub struct FrameBuffer<'a> { + base: *mut u8, + size: usize, + _lifetime: PhantomData<&'a mut u8>, +} + +impl<'a> FrameBuffer<'a> { + /// Access the raw framebuffer pointer + /// + /// To use this pointer safely and correctly, you must... + /// - Honor the pixel format and stride specified by the mode info + /// - Keep memory accesses in bound + /// - Use volatile reads and writes + /// - Make sure that the pointer does not outlive the FrameBuffer + pub fn as_mut_ptr(&mut self) -> *mut u8 { + self.base + } + + /// Query the framebuffer size in bytes + pub fn size(&self) -> usize { + self.size + } + + /// Modify the i-th byte of the frame buffer + /// + /// This operation is unsafe because... + /// - You must honor the pixel format and stride specified by the mode info + /// - There is no bound checking on memory accesses in release mode + #[inline] + pub unsafe fn write_byte(&mut self, index: usize, value: u8) { + debug_assert!(index < self.size, "Frame buffer accessed out of bounds"); + self.base.add(index).write_volatile(value) + } + + /// Read the i-th byte of the frame buffer + /// + /// This operation is unsafe because... + /// - You must honor the pixel format and stride specified by the mode info + /// - There is no bound checking on memory accesses in release mode + #[inline] + pub unsafe fn read_byte(&self, index: usize) -> u8 { + debug_assert!(index < self.size, "Frame buffer accessed out of bounds"); + self.base.add(index).read_volatile() + } + + /// Write a value in the frame buffer, starting at the i-th byte + /// + /// We only recommend using this method with [u8; N] arrays. Once Rust has + /// const generics, it will be deprecated and replaced with a write_bytes() + /// method that only accepts [u8; N] input. + /// + /// This operation is unsafe because... + /// - It is your reponsibility to make sure that the value type makes sense + /// - You must honor the pixel format and stride specified by the mode info + /// - There is no bound checking on memory accesses in release mode + #[inline] + pub unsafe fn write_value(&mut self, index: usize, value: T) { + debug_assert!( + index.saturating_add(mem::size_of::()) <= self.size, + "Frame buffer accessed out of bounds" + ); + (self.base.add(index) as *mut T).write_volatile(value) + } + + /// Read a value from the frame buffer, starting at the i-th byte + /// + /// We only recommend using this method with [u8; N] arrays. Once Rust has + /// const generics, it will be deprecated and replaced with a read_bytes() + /// method that only accepts [u8; N] input. + /// + /// This operation is unsafe because... + /// - It is your reponsibility to make sure that the value type makes sense + /// - You must honor the pixel format and stride specified by the mode info + /// - There is no bound checking on memory accesses in release mode + #[inline] + pub unsafe fn read_value(&self, index: usize) -> T { + debug_assert!( + index.saturating_add(mem::size_of::()) <= self.size, + "Frame buffer accessed out of bounds" + ); + (self.base.add(index) as *const T).read_volatile() + } +} diff --git a/uefi-test-runner/src/proto/console/gop.rs b/uefi-test-runner/src/proto/console/gop.rs index 21143ee76..f07536698 100644 --- a/uefi-test-runner/src/proto/console/gop.rs +++ b/uefi-test-runner/src/proto/console/gop.rs @@ -1,5 +1,5 @@ use uefi::prelude::*; -use uefi::proto::console::gop::{BltOp, BltPixel, GraphicsOutput, PixelFormat}; +use uefi::proto::console::gop::{BltOp, BltPixel, FrameBuffer, GraphicsOutput, PixelFormat}; use uefi::table::boot::BootServices; use uefi_exts::BootServicesExt; @@ -54,20 +54,14 @@ fn draw_fb(gop: &mut GraphicsOutput) { let stride = mi.stride(); let (width, height) = mi.resolution(); - let (fb_base, _fb_size) = gop.frame_buffer(); + let mut fb = gop.frame_buffer(); - type PixelWriter = unsafe fn(*mut u8, [u8; 3]); - unsafe fn write_pixel_rgb(pixel_base: *mut u8, rgb: [u8; 3]) { - let [r, g, b] = rgb; - pixel_base.add(0).write_volatile(r); - pixel_base.add(1).write_volatile(g); - pixel_base.add(2).write_volatile(b); + type PixelWriter = unsafe fn(&mut FrameBuffer, usize, [u8; 3]); + unsafe fn write_pixel_rgb(fb: &mut FrameBuffer, pixel_base: usize, rgb: [u8; 3]) { + fb.write_value(pixel_base, rgb); }; - unsafe fn write_pixel_bgr(pixel_base: *mut u8, rgb: [u8; 3]) { - let [r, g, b] = rgb; - pixel_base.add(0).write_volatile(b); - pixel_base.add(1).write_volatile(g); - pixel_base.add(2).write_volatile(r); + unsafe fn write_pixel_bgr(fb: &mut FrameBuffer, pixel_base: usize, rgb: [u8; 3]) { + fb.write_value(pixel_base, [rgb[2], rgb[1], rgb[0]]); }; let write_pixel: PixelWriter = match mi.pixel_format() { PixelFormat::RGB => write_pixel_rgb, @@ -78,15 +72,15 @@ fn draw_fb(gop: &mut GraphicsOutput) { } }; - let fill_rectangle = |(x1, y1), (x2, y2), color| { + let mut fill_rectangle = |(x1, y1), (x2, y2), color| { assert!((x1 < width) && (x2 < width), "Bad X coordinate"); assert!((y1 < height) && (y2 < height), "Bad Y coordinate"); for row in y1..y2 { for column in x1..x2 { unsafe { - let index = (row * stride) + column; - let pixel_base = fb_base.add(4 * index); - write_pixel(pixel_base, color); + let pixel_index = (row * stride) + column; + let pixel_base = 4 * pixel_index; + write_pixel(&mut fb, pixel_base, color); } } }