uefi: serial: various improvements to doc, read(), and write() #1852

phip1611 · 2025-12-27T21:25:41Z

This improves multiple things for the serial IO protocol. Please review commit by commit.

I tested this in a VM as well as on real hardware. The changes make the Serial IO protocol more usable and also help to better cope with non-spec compliant implementations.

Checklist

Sensible git history (for example, squash "typo" or "fix" commits). See the Rewriting History guide for help.
Update the changelog (if necessary)

uefi/CHANGELOG.md

phip1611 · 2025-12-28T11:46:50Z

Please review again, @nicholasbishop! I changed the PR since your last review quite significantly.

PS: I'm working on a small fun project with the serial device in UEFI.

uefi-raw/src/protocol/console/serial.rs

uefi/src/proto/console/serial.rs

nicholasbishop · 2026-01-01T17:48:37Z

uefi/src/proto/console/serial.rs

+    #[cfg(feature = "alloc")]
+    pub fn read_to_end(&mut self) -> Result<Vec<u8>> {
+        let mut vec = Vec::new();
+        let mut buf = [0; 2048];


Rather than reading into this temporary buffer and copying it to the vec, could we write directly to the vec?

good question! no we could not unfortunately, because then we would lose the ability to grow the vector dynamically.

I'm not clear why. Couldn't we write the vec, resize if needed, and then write at an offset for the next iteration, and so on?

I refactored that part - I just learned about https://doc.rust-lang.org/std/vec/struct.Vec.html#method.spare_capacity_mut which did the trick

nicholasbishop · 2026-01-01T17:50:20Z

uefi/src/proto/console/serial.rs

    pub fn write(&mut self, data: &[u8]) -> Result<(), usize> {
        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()),


Is removing this debug_assert intentional? I don't see anything wrong with it.

I decided to remove that intentionally as the implementation may decide to only write some bytes and one must call the function again afterwards. https://uefi.org/specs/UEFI/2.10/12_Protocols_Console_Support.html#efi-serial-io-protocol-write

That would seem to conflict with this function's docstring: "This operation will block until the data has been fully written or an error occurs." If the docstring is correct, than the assert can stay, otherwise we should fix the docstring as well. I find the UEFI spec a little ambiguous in its language here. "EFI_SUCCESS -- The data was written" could be taken to mean all the data was written, but I'm not sure. Did you observe that calling write multiple times is needed in some real-world test?

I decided to keep the assert. I actually upgraded the debug asserts to runtime asserts. If the protocol impl behaves unexpectedly, the program should at least notice this

phip1611 · 2026-01-01T22:20:19Z

I refactored the PR and hope that I now improve the situation in a more flexible way.

We are logically modifying state, so this should be mut.

Next to documentation improvements, I replaced the debug assertions with runtime assertions. If the underlying protocol implementation doesn't behave as expected, we should at least fail hard. I tested this with OVMF and also on real hardware.

This is the more expected and natural behavior.

nicholasbishop

Adding a few more comments. I haven't fully checked the rest though; I think it would be good to try to split this PR up since there are a lot of changes to consider here. Perhaps one PR for the uefi-raw changes, one for the docstring improvements to existing functions, one each for the changes to read and write, and a couple more for read_to_vec/write_exact.

nicholasbishop · 2026-01-25T06:06:10Z

uefi-raw/src/protocol/console/serial.rs

 }

+newtype_enum! {
+    /// Type of queues an NVMe command can be placed into


Wrong docstring here

nicholasbishop · 2026-01-25T06:08:45Z

uefi-raw/src/protocol/console/serial.rs

+        REVISION_0 = 0x00010000,
+        /// Version 1.1.
+        REVISION_1P1 = 0x00010001,


Suggested change

REVISION_0 = 0x00010000,

/// Version 1.1.

REVISION_1P1 = 0x00010001,

REVISION_1_0 = 0x00010000,

/// Version 1.1.

REVISION_1_1 = 0x00010001,

I recognize this drifts a little from the spec names, but it matches how we've defined such constants elsewhere. example

nicholasbishop · 2026-01-25T06:09:35Z

uefi-test-runner/src/main.rs


    // Send a screenshot request to the host.
-    serial.write(request.as_bytes()).discard_errdata()?;
+    //serial.write(request.as_bytes()).discard_errdata()?;


Suggested change

//serial.write(request.as_bytes()).discard_errdata()?;

nicholasbishop · 2026-01-25T06:20:36Z

uefi-raw/src/protocol/console/serial.rs

    pub read: unsafe extern "efiapi" fn(*mut Self, *mut usize, *mut u8) -> Status,
    pub mode: *const SerialIoMode,
+    // Revision 1.1
+    pub device_type_guid: *const Guid,


Something to consider here is potential UB from the way we use this type in the uefi crate. We wrap SerialIoProtocol into the Serial type. Via ScopedProtocol we allow creating a &Serial. If the underlying C protocol revision is 1.0, this field won't exist, so we'll have a reference to a type that is larger than the underlying C allocation. Even if we never read from the device_type_guid field (i.e. by first checking the revision), I'm pretty sure that's still UB.

phip1611 · 2026-01-25T08:21:17Z

; I think it would be good to try to split this PR up since there are a lot of changes to consider here.

Yes, I thought so as well. This PR grew larger than anticipated.
Closing this in favor of multiple smaller ones.

phip1611 self-assigned this Dec 27, 2025

nicholasbishop approved these changes Dec 28, 2025

View reviewed changes

nicholasbishop reviewed Dec 28, 2025

View reviewed changes

uefi/CHANGELOG.md Outdated Show resolved Hide resolved

phip1611 force-pushed the serial-read branch from 19e0b16 to b70296c Compare December 28, 2025 11:43

phip1611 changed the title ~~uefi: serial: fix read()~~ uefi: serial: improve read() Dec 28, 2025

phip1611 requested a review from nicholasbishop December 28, 2025 11:45

phip1611 force-pushed the serial-read branch 2 times, most recently from bba5e4f to 9218d96 Compare December 28, 2025 11:49

nicholasbishop reviewed Jan 1, 2026

View reviewed changes

phip1611 force-pushed the serial-read branch 3 times, most recently from 10c84c2 to b54e7fe Compare January 1, 2026 22:19

phip1611 requested a review from nicholasbishop January 1, 2026 22:19

phip1611 force-pushed the serial-read branch from b54e7fe to 5a62081 Compare January 1, 2026 22:20

phip1611 force-pushed the serial-read branch from 5a62081 to 402596f Compare January 1, 2026 22:25

uefi-raw: serial: fix mutability

041f28b

We are logically modifying state, so this should be mut.

phip1611 marked this pull request as draft January 24, 2026 11:41

phip1611 added 2 commits January 24, 2026 14:48

uefi-raw: extend support for revision in SERIAL_IO_PROTOCOL

20297b9

uefi: serial: improve clarity over timeout in SerialIoMode

f1edb3e

phip1611 force-pushed the serial-read branch from 402596f to 9322e57 Compare January 24, 2026 17:03

phip1611 changed the title ~~uefi: serial: improve read()~~ uefi: serial: various improvements to doc, read(), and write() Jan 24, 2026

phip1611 marked this pull request as ready for review January 24, 2026 17:04

uefi: serial: add support for revision

678394f

phip1611 force-pushed the serial-read branch from 9322e57 to 537342e Compare January 24, 2026 17:10

phip1611 added 3 commits January 24, 2026 20:56

uefi: serial: improve overall documentation

9d3a770

uefi: serial: improve read() and write()

11db5ff

Next to documentation improvements, I replaced the debug assertions with runtime assertions. If the underlying protocol implementation doesn't behave as expected, we should at least fail hard. I tested this with OVMF and also on real hardware.

uefi: serial: add read_exact() and write_exact()

1a31b17

phip1611 force-pushed the serial-read branch from 537342e to 8e9a2b9 Compare January 24, 2026 19:58

phip1611 added 2 commits January 24, 2026 21:01

uefi: serial: read_to_vec()

32172b7

uefi: serial: fix core::fmt::Write impl

9098df9

This is the more expected and natural behavior.

phip1611 force-pushed the serial-read branch from 8e9a2b9 to 9098df9 Compare January 24, 2026 20:01

nicholasbishop reviewed Jan 25, 2026

View reviewed changes

phip1611 closed this Jan 25, 2026

This was referenced Jan 25, 2026

uefi-raw: serial: fix mutability #1872

Open

uefi-raw & uefi: add support for protocol revision 1.1 #1873

Open

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

Open

Uh oh!

uefi: serial: various improvements to doc, read(), and write() #1852

uefi: serial: various improvements to doc, read(), and write() #1852

Conversation

phip1611 commented Dec 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

Uh oh!

phip1611 commented Dec 28, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phip1611 Jan 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phip1611 Jan 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phip1611 commented Jan 1, 2026

Uh oh!

nicholasbishop left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phip1611 commented Jan 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

phip1611 commented Dec 27, 2025 •

edited

Loading

phip1611 Jan 1, 2026 •

edited

Loading

phip1611 Jan 24, 2026 •

edited

Loading

phip1611 commented Jan 25, 2026 •

edited

Loading