Skip to content

deps: bump uart_16550 to 0.6.0#565

Merged
Freax13 merged 2 commits into
rust-osdev:mainfrom
phip1611:uart
Jun 16, 2026
Merged

deps: bump uart_16550 to 0.6.0#565
Freax13 merged 2 commits into
rust-osdev:mainfrom
phip1611:uart

Conversation

@phip1611

@phip1611 phip1611 commented Jun 14, 2026

Copy link
Copy Markdown
Member

Let's use the latest and greatest version

@phip1611 phip1611 self-assigned this Jun 14, 2026
@phip1611 phip1611 changed the title deps: bump uart_16550 to 0.6.0 + streamline as workspace dep deps: bump uart_16550 to 0.6.0 Jun 14, 2026
@phip1611 phip1611 requested a review from phil-opp June 14, 2026 19:51
@phip1611 phip1611 requested a review from Freax13 June 14, 2026 19:54
@phip1611 phip1611 force-pushed the uart branch 2 times, most recently from 69442f7 to 21dd2d3 Compare June 14, 2026 20:28
Comment thread tests/test_kernels/min_stack/src/bin/basic_boot.rs Outdated

@Freax13 Freax13 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution!

Comment thread common/src/serial.rs
use core::fmt;
use uart_16550::backend::PioBackend;

pub struct SerialPort {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we replace SerialPort with Uart16550Tty? Both implement core::fmt::Write, so I don't see much of a reason to keep SerialPort around. Uart16550Tty also does the \n to \r\n conversion SerialPort does.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried but for some reason on of these checks fails:

image

This surprises me becaue in the integration test of uart_16550 it works in QEMU. I don't have time to investigate for now. Can we move forward?

I've added a TODO comment in the code

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks to me like it's spinning somewhere. I think it's spinning in receive_bytes_exact.

@phip1611 phip1611 Jun 17, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you help me investigating please? I'd love to understand why it fails

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you help me investigating please? I'd love to understand why it fails

I tried looking into this yesterday. I think it's some kind of race condition in the self-test code. I can't trigger the issue reliably though. Adding artificial system load seems to make it easier to reproduce, but it's still not completely reliable.
For some reason, I had a harder time reproducing the error with the latest HEAD for uart_16550. I'm not sure why.
I might be able to take another look at this later today.
I think my next step is going to be looking at the serial port implementation in QEMU to see if there's anything that could explain the race condition (maybe asynchronous processing?).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know about you, but I've only seen this happen when booting with UEFI. Maybe UEFI's initialization and use of the serial port before the bootloader somehow causes issues?

@phip1611 phip1611 Jun 17, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting - the UEFI console takes ownership of the serial device by default. One must therefore disconnect the UEFI console from using the serial device (as long as boot services are active but one wants to use the serial device). I think this is the problem.

The forced disconnct is also fairly easy

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The forced disconnct is also fairly easy

How would one do that?


Here's something interesting: Wrapping the call to Uart16550Tty::new_port in x86_64::instructions::interrupts::without_interrupts fixes the issue. Disabling interrupts in the tty config (config.interrupts = IER::empty();) does not work however.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The forced disconnct is also fairly easy

How would one do that?

https://github.com/phip1611/uefi-serial-chat/blob/47cc3961ab047d698790dc48131799aa10c18c65/src/main.rs#L104

Regarding interrupts:thanks! That's something I can use for my testing and debugging

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW I'm preparing an update of the uefi dependency

Comment thread tests/test_kernels/min_stack/src/bin/basic_boot.rs Outdated
phip1611 added 2 commits June 16, 2026 08:07
Uart16550 has a layout of (32 /* size*/, 4 /* align*/) whereas the old
SerialPort had (2, 2). The test shows that this small change is
sufficient to fail the test with the old stack size. We slightly increase
the stack therefore.

@Freax13 Freax13 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@Freax13 Freax13 merged commit 617e92c into rust-osdev:main Jun 16, 2026
9 checks passed
@phip1611 phip1611 deleted the uart branch June 17, 2026 05:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants