uefi: serial: improve read() and write(), add read_exact() and write_exact(), add read_to_end()

uefi-test-runner/src/main.rs

@@ -10,6 +10,7 @@ extern crate alloc;
 
 use alloc::string::ToString;
 use alloc::vec::Vec;
+use core::fmt::Write;
 use uefi::mem::memory_map::MemoryMap;
 use uefi::prelude::*;
 use uefi::proto::console::serial::Serial;
@@ -115,7 +116,10 @@ fn send_request_helper(serial: &mut Serial, request: HostRequest) -> Result {
     serial.set_attributes(&io_mode)?;
 
     // Send a screenshot request to the host.
-    serial.write(request.as_bytes()).discard_errdata()?;
+    //serial.write(request.as_bytes()).discard_errdata()?;
+    serial
+        .write_str(request.as_str())
+        .expect("should write all bytes");
 
     // Wait for the host's acknowledgement before moving forward.
     let mut reply = [0; 3];

uefi/CHANGELOG.md

@@ -7,6 +7,8 @@
 - Added `proto::pci::root_bridge::PciRootBridgeIo::enumerate()`.
 - Added `proto::nvme::pass_thru::NvmePassThru::broadcast()`.
 - Added `proto::media::block::BlockIO2`.
+- Added `Serial::read_exact()` and `Serial::write_exact()`
+- Added `Serial::read_to_vec()`
 
 ## Changed
 - export all `text::{input, output}::*` types
@@ -24,6 +26,9 @@
 - **Breaking:** `boot::check_event` now consumes `&Event` rather than `Event`, removing the
   need for unnecessary `Event::unsafe_clone()`s.
 - MSRV increased to 1.88.
+- `Serial::read()` and `Serial::write()` got small improvements to better
+  match the spec. Read the documentation of these methods for further insights.
+- Fixed potential partial writes in `core::fmt::Write` impl of `Serial` protocol
 
 # uefi - v0.36.1 (2025-11-05)
 

uefi/src/proto/console/serial.rs

@@ -2,16 +2,19 @@
 
 //! Abstraction over byte stream devices, also known as serial I/O devices.
 
-#[cfg(doc)]
-use crate::Status;
 use crate::proto::unsafe_protocol;
-use crate::{Result, StatusExt};
+use crate::{Error, Result, ResultExt, Status, StatusExt};
+use core::fmt;
 use core::fmt::Write;
+use core::time::Duration;
+use log::error;
+use uefi::boot;
 use uefi_raw::protocol::console::serial::SerialIoProtocol;
-
 pub use uefi_raw::protocol::console::serial::{
     ControlBits, Parity, SerialIoMode as IoMode, StopBits,
 };
+#[cfg(feature = "alloc")]
+use {alloc::vec::Vec, core::slice};
 
 /// Serial IO [`Protocol`]. Provides access to a serial I/O device.
 ///
@@ -124,35 +127,205 @@ impl Serial {
         unsafe { (self.0.set_control_bits)(&mut self.0, bits) }.to_result()
     }
 
-    /// Reads data from this device.
+    /// Reads data from the device. This function has the raw semantics of the
+    /// underlying UEFI protocol.
     ///
-    /// This operation will block until the buffer has been filled with data or
-    /// an error occurs. In the latter case, the error will indicate how many
-    /// bytes were actually read from the device.
-    pub fn read(&mut self, data: &mut [u8]) -> Result<(), usize> {
-        let mut buffer_size = data.len();
-        unsafe { (self.0.read)(&mut self.0, &mut buffer_size, data.as_mut_ptr()) }.to_result_with(
-            || debug_assert_eq!(buffer_size, data.len()),
+    /// The function will read bytes until either the buffer is full or a
+    /// timeout or overrun error occurs.
+    ///
+    /// # Arguments
+    ///
+    /// - `buffer`: buffer to fill
+    ///
+    /// # Tips
+    ///
+    /// Consider setting non-default properties via [`Self::set_attributes`]
+    /// and [`Self::set_control_bits`] matching your use-case. For more info,
+    /// please read the general [documentation](Self) of the protocol.
+    ///
+    /// # Errors
+    ///
+    /// - [`Status::DEVICE_ERROR`]: serial device reported an error
+    /// - [`Status::TIMEOUT`]: operation was stopped due to a timeout or overrun
+    pub fn read(&mut self, buffer: &mut [u8]) -> Result<(), usize /* read bytes on timeout*/> {
+        let mut buffer_size = buffer.len();
+        unsafe { (self.0.read)(&mut self.0, &mut buffer_size, buffer.as_mut_ptr()) }.to_result_with(
+            || {
+                // By spec: Either reads all requested bytes (and blocks) or
+                // returns early with an error.
+                assert_eq!(buffer_size, buffer.len())
+            },
             |_| buffer_size,
         )
     }
 
-    /// Writes data to this device.
+    /// Variant of [`Self::read`] that blocks until the exact amount of
+    /// requested bytes were read.
+    ///
+    /// This loops over potential [`Status::TIMEOUT`]s and just tries again. It
+    /// is unlikely that this causes and endless loop, as in normal operation we
+    /// expect the device to make forward progress.
+    ///
+    /// # Arguments
+    ///
+    /// - `buffer`: buffer to fill
+    ///
+    /// # Tips
+    ///
+    /// Consider setting non-default properties via [`Self::set_attributes`]
+    /// and [`Self::set_control_bits`] matching your use-case. For more info,
+    /// please read the general [documentation](Self) of the protocol.
+    ///
+    /// # Errors
+    ///
+    /// - [`Status::DEVICE_ERROR`]: serial device reported an error
+    pub fn read_exact(&mut self, buffer: &mut [u8]) -> Result<()> {
+        let mut remaining_buffer = buffer;
+        // We retry on timeout and only return other errors
+        while !remaining_buffer.is_empty() {
+            match self.read(remaining_buffer) {
+                Ok(_) => {
+                    break;
+                }
+                Err(err) if err.status() == Status::TIMEOUT => {
+                    let n = *err.data();
+                    remaining_buffer = &mut remaining_buffer[n..];
+                    boot::stall(Duration::from_millis(1));
+                }
+                err => {
+                    return Err(Error::from(err.status()));
+                }
+            }
+        }
+        Ok(())
+    }
+
+    /// Reads all data from the device until the first timeout occurs.
+    ///
+    /// # Tips
+    ///
+    /// Consider setting non-default properties via [`Self::set_attributes`]
+    /// and [`Self::set_control_bits`] matching your use-case. For more info,
+    /// please read the general [documentation](Self) of the protocol.
+    ///
+    /// # Errors
+    ///
+    /// - [`Status::DEVICE_ERROR`]: serial device reported an error
+    #[cfg(feature = "alloc")]
+    pub fn read_to_vec(&mut self) -> Result<Vec<u8>> {
+        let mut vec = Vec::new();
+        loop {
+            vec.reserve(256);
+
+            let spare = vec.spare_capacity_mut();
+
+            // SAFETY: `read` promises to write `n` bytes starting at
+            // `spare.as_mut_ptr()` and `n <= buf.len()`.
+            let spare_slice =
+                unsafe { slice::from_raw_parts_mut(spare.as_mut_ptr().cast::<u8>(), spare.len()) };
+
+            let n = match self.read(spare_slice) {
+                Ok(_) => spare_slice.len(),
+                Err(e) if e.status() == Status::TIMEOUT => *e.data(),
+                Err(e) => return Err(Error::from(e.status())),
+            };
+
+            // SAFETY: We know how many bytes have just been written.
+            unsafe {
+                vec.set_len(vec.len() + n);
+            }
+            if n == 0 {
+                break;
+            }
+        }
+        Ok(vec)
+    }
+
+    /// Writes data to this device. This function has the raw semantics of the
+    /// underlying UEFI protocol.
     ///
-    /// This operation will block until the data has been fully written or an
-    /// error occurs. In the latter case, the error will indicate how many bytes
-    /// were actually written to the device.
-    pub fn write(&mut self, data: &[u8]) -> Result<(), usize> {
+    /// The function will try to write all provided bytes in the configured
+    /// timeout.
+    ///
+    /// # Arguments
+    ///
+    /// - `data`: bytes to write
+    ///
+    /// # Tips
+    ///
+    /// Consider setting non-default properties via [`Self::set_attributes`]
+    /// and [`Self::set_control_bits`] matching your use-case. For more info,
+    /// please read the general [documentation](Self) of the protocol.
+    ///
+    /// # Errors
+    ///
+    /// - [`Status::DEVICE_ERROR`]: serial device reported an error
+    /// - [`Status::TIMEOUT`]: data write was stopped due to a timeout
+    pub fn write(&mut self, data: &[u8]) -> Result<(), usize /* bytes written on timeout */> {
         let mut buffer_size = data.len();
         unsafe { (self.0.write)(&mut self.0, &mut buffer_size, data.as_ptr()) }.to_result_with(
-            || debug_assert_eq!(buffer_size, data.len()),
+            || {
+                // By spec: Either reads all requested bytes (and blocks) or
+                // returns early with an error.
+                assert_eq!(buffer_size, data.len())
+            },
             |_| buffer_size,
         )
     }
+
+    /// Variant of [`Self::write`] that blocks until the exact number of
+    /// provided bytes were written.
+    ///
+    /// This loops over potential [`Status::TIMEOUT`]s and just tries again. It
+    /// is unlikely that this causes and endless loop, as in normal operation we
+    /// expect the device to make forward progress.
+    ///
+    /// # Arguments
+    ///
+    /// - `data`: bytes to write
+    ///
+    /// # Tips
+    ///
+    /// Consider setting non-default properties via [`Self::set_attributes`]
+    /// and [`Self::set_control_bits`] matching your use-case. For more info,
+    /// please read the general [documentation](Self) of the protocol.
+    ///
+    /// # Errors
+    ///
+    /// - [`Status::DEVICE_ERROR`]: serial device reported an error
+    pub fn write_exact(&mut self, data: &[u8]) -> Result<()> {
+        let mut remaining_bytes = data;
+        // We retry on timeout and only return other errors
+        while !remaining_bytes.is_empty() {
+            match self.write(remaining_bytes) {
+                Ok(_) => {
+                    break;
+                }
+                Err(err) if err.status() == Status::TIMEOUT => {
+                    let n = *err.data();
+                    remaining_bytes = &remaining_bytes[n..];
+                    boot::stall(Duration::from_millis(1));
+                }
+                err => {
+                    return Err(Error::from(err.status()));
+                }
+            }
+        }
+        Ok(())
+    }
 }
 
 impl Write for Serial {
-    fn write_str(&mut self, s: &str) -> core::fmt::Result {
-        self.write(s.as_bytes()).map_err(|_| core::fmt::Error)
+    fn write_str(&mut self, s: &str) -> fmt::Result {
+        // We retry on Status::TIMEOUT but propagate other errors
+        self.write_exact(s.as_bytes()).map_err(|e| {
+            let msg = "failed to write to device";
+            // Simple check to prevent endless recursion if a logger
+            // implementation uses the serial protocol
+            if !s.starts_with(msg) {
+                error!("{msg}: {e}");
+            }
+            fmt::Error
+        })
     }
 }