Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix logger connection after opening serial protocol #1031

Merged
merged 3 commits into from
Dec 19, 2023

Conversation

nicholasbishop
Copy link
Member

My recent PR to upgrade the edk2 prebuilts had a bug I didn't notice: the logging output gets cut off after the serial protocol is opened in exclusive mode.

This was caused by tianocore/edk2@cf6a0a5. Fix it by specifying the appropriate terminal type when calling connect_controller.

Since this bug has happened in various forms a couple times, also update the tests to catch it: check for a special log message right before exiting boot services, and fail the tests if that message is not received.

Checklist

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

@nicholasbishop nicholasbishop force-pushed the bishop-fix-output-again branch from 5049580 to faafb18 Compare December 19, 2023 00:04
These are from section "Vendor-Defined Messaging Device Path" in the UEFI spec.
After opening the serial protocol in exclusive mode we have to reconnect it to
the output console or we'll drop logs. After the recent upgrade to edk2 this
code stopped working. Fix it by specifying the terminal type when calling
`connect_controller`.

This is needed because of edk2 commit cf6a0a52b07: "use utf8 for the serial
console". The default terminal type (for OVMF) was changed from PC-ANSI to
UTF-8. However, in TerminalDriverBindingStart, the terminal type will be reset
to the default value of PC-ANSI if `remaining_device_path` is null. That in turn
will prevent the code from finding the VT-UTF8 device.

The terminal type on aarch64 is VT 100, so switch based on the target aarch.
We've had a few bugs where logging stopped working at some point during the
tests. It's easy to miss that if you aren't paying careful attention to the log
output, so add a test for it.
@nicholasbishop nicholasbishop force-pushed the bishop-fix-output-again branch from faafb18 to 18a01a3 Compare December 19, 2023 00:04
Copy link
Contributor

@phip1611 phip1611 left a comment

Choose a reason for hiding this comment

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

Nice, LGTM

@phip1611 phip1611 added this pull request to the merge queue Dec 19, 2023
Merged via the queue into rust-osdev:main with commit addd6e1 Dec 19, 2023
12 checks passed
@nicholasbishop nicholasbishop deleted the bishop-fix-output-again branch December 19, 2023 16:27
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