Page MenuHomeFreeBSD

loader: Fully reset terminal settings, not just colors
Needs ReviewPublic

Authored by ryan_freqlabs.com on Sep 20 2019, 7:42 PM.

Details

Reviewers
kevans
tsoome
Summary

Early boot firmware or other software may leave serial terminals in an unspecified, potentially broken state at boot. This can include problems such as unfavorable color settings or fixed scrolling regions which make the loader menu and later kernel messages unreadable.

We currently reset terminal color settings using an ANSI escape sequence when loader colors are enabled. Instead, we should use the \ec escape sequence to reset all of the terminal settings. This should fix a broader range of console visibility issues.

Sponsored by: iXsystems, Inc.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

kevans accepted this revision.Sep 20 2019, 7:48 PM

Another OK from lua side

This revision is now accepted and ready to land.Sep 20 2019, 7:48 PM
tsoome accepted this revision.Sep 20 2019, 7:49 PM
kevans added a subscriber: emaste.Sep 20 2019, 7:50 PM

Hmm... on second thought, this might need tested with a black-on-white xterm. I seem to recall having to spit out color.default() because we couldn't cope with the different background color and kept screwing up @emaste's xterm. I'm assuming this will put us back into that state, and we'll need to have the color.default() back after the reset.

Hmm... on second thought, this might need tested with a black-on-white xterm. I seem to recall having to spit out color.default() because we couldn't cope with the different background color and kept screwing up @emaste's xterm. I'm assuming this will put us back into that state, and we'll need to have the color.default() back after the reset.

the terminal reset means you will get the terminal defaults - if the xterm was set to black on white, it will have black on white after reset. Now the loader teken is built white on black and we have no toggle to set teken defaults (yet) - so if you did set fg/bg colors for local console, then after terminal reset you will get white on black.

ryan_freqlabs.com planned changes to this revision.Sep 20 2019, 8:23 PM

Sorry, I do need to take a second look at this one. My terminal doesn't seem to be resetting the font brightness correctly when loader_color=YES in a bhyve VM looking at the serial console. Everything looks bolded until I reset the terminal again in the shell using `printf '\033c' and then it goes back to normal.

ryan_freqlabs.com updated this revision to Diff 62482.EditedSep 23 2019, 7:40 PM

I moved the UEFI console resize to before the screen reset in loader.lua to be consistent with the order of this sequence in the forth loader.rc version. In doing so I noticed a few if blocks for the menu could be combined, so I combined them.

The odd brightness issue I had noticed earlier was fixed by @tsoome over the weekend in rS352601, thanks!

kevans added inline comments.Sep 23 2019, 8:15 PM
stand/lua/loader.lua
54

I'd consider going a step further and eliminating the single-use 'menu' local and instead just invoking require("menu").run(). I really don't recall why I split this up... trolling through the history doesn't explain much.

Eliminate menu local per review.

ryan_freqlabs.com marked an inline comment as done.Sep 23 2019, 8:27 PM
ryan_freqlabs.com added a comment.EditedSep 24 2019, 8:01 PM

I was a bit slow taking the screenshot and caught the menu mid-scroll but here it is in a black on white terminal:

I must ask for someone with a commit bit to put this one in for me as well, if there are no further concerns. Thanks in advance!

imp added a comment.Sep 25 2019, 10:07 PM

Do we need this now that tsoome has committed all his stuff to fix the broken color stuff that lead to this situation in the first place?

ryan_freqlabs.com added a comment.EditedSep 25 2019, 10:20 PM

This aims to reset more of the state to defaults, not just colors. Things like the scroll region for example, so we don't have all the output stuck on one line. There are a multitude of software/firmware/hardware issues that can cause the state to be unfavorable at boot. Loose serial connections, buggy firmware, etc. tsoome's work complements this change rather than invalidating it.

imp added a comment.Sep 25 2019, 10:50 PM

Does this work for EFI console? When I tried it, the terminal emualtor intercepted the escape sequences and so the serial console didn't get them...

imp added a comment.Sep 25 2019, 11:13 PM

I had very similar code in FORTH for our boot loader and it didn't work with EFI, just comconsole. while we've transitioned to teken, it looks like it eats the ESC c and does a reset for the eficonsole, which means that efi's com redirect in the BIOS doesn't work.

stand/forth/screen.4th
43

The reset sequence is ESC c, not ESC [ c.
https://en.wikipedia.org/wiki/ANSI_escape_code

stand/lua/loader.lua
50

Why only reset of if color is enabled if the goal is to also reset scrolling regions?

while we've transitioned to teken, it looks like it eats the ESC c

Ok, that looks pretty tough to deal with. Now that you mention it, I'm not seeing colors on the EFI serial console either, and under bhyve the scroll region is broken after the loader hands off to the kernel as well. It's everything I wanted to fix, still broken! Ugh! I thought I had tested that, but I guess I only tested a video output with the EFI console.

One way of dealing with this could be to add a variable to bypass teken for users of serial EFI consoles. A more challenging but perhaps more flexible option would be to implement a driver for the EFI serial protocol and either expose it as something like "efiserial" console or maybe add special handling to the existing EFI console driver to duplicate ANSI sequences to the serial port.

I don't think solving that problem with the EFI console/teken is within the scope of this change, though I am certainly hoping it gets fixed. I've only learned of teken's existence a few days ago. Can someone more familiar with that piece provide their input on the issue?

stand/forth/screen.4th
43

Oops, I will change this to

: ris ( -- ) 27 emit [char] c emit ;
stand/lua/loader.lua
50

To not bother people who don't want to see ANSI escape sequences. Making the set of loader options even more complicated seemed less user friendly.

imp added a comment.Sep 26 2019, 3:08 AM

while we've transitioned to teken, it looks like it eats the ESC c

Ok, that looks pretty tough to deal with. Now that you mention it, I'm not seeing colors on the EFI serial console either, and under bhyve the scroll region is broken after the loader hands off to the kernel as well. It's everything I wanted to fix, still broken! Ugh! I thought I had tested that, but I guess I only tested a video output with the EFI console.

It is the most maddening part to get right... It's why I never upstreamed my hacks that I knew didn't work.

One way of dealing with this could be to add a variable to bypass teken for users of serial EFI consoles. A more challenging but perhaps more flexible option would be to implement a driver for the EFI serial protocol and either expose it as something like "efiserial" console or maybe add special handling to the existing EFI console driver to duplicate ANSI sequences to the serial port.

I actually think that's quite hard and involved. The current code doesn't differentiate, unfortunately. It would take a little while to understand a good path forward. At the moment, I see parsing ConOut, looking up handles and using that in something complicated, but there may be something simpler...

I don't think solving that problem with the EFI console/teken is within the scope of this change, though I am certainly hoping it gets fixed. I've only learned of teken's existence a few days ago. Can someone more familiar with that piece provide their input on the issue?

This can really only work for comconsole. And even there it can only be imperfect. And people will complain and expect me to fix it, so I'm reluctant to sign off until we have a better notion of a path forward...

In D21733#475785, @imp wrote:

while we've transitioned to teken, it looks like it eats the ESC c

Ok, that looks pretty tough to deal with. Now that you mention it, I'm not seeing colors on the EFI serial console either, and under bhyve the scroll region is broken after the loader hands off to the kernel as well. It's everything I wanted to fix, still broken! Ugh! I thought I had tested that, but I guess I only tested a video output with the EFI console.

It is the most maddening part to get right... It's why I never upstreamed my hacks that I knew didn't work.

One way of dealing with this could be to add a variable to bypass teken for users of serial EFI consoles. A more challenging but perhaps more flexible option would be to implement a driver for the EFI serial protocol and either expose it as something like "efiserial" console or maybe add special handling to the existing EFI console driver to duplicate ANSI sequences to the serial port.

I actually think that's quite hard and involved. The current code doesn't differentiate, unfortunately. It would take a little while to understand a good path forward. At the moment, I see parsing ConOut, looking up handles and using that in something complicated, but there may be something simpler...

I don't think solving that problem with the EFI console/teken is within the scope of this change, though I am certainly hoping it gets fixed. I've only learned of teken's existence a few days ago. Can someone more familiar with that piece provide their input on the issue?

This can really only work for comconsole. And even there it can only be imperfect. And people will complain and expect me to fix it, so I'm reluctant to sign off until we have a better notion of a path forward...

I am investigating this already, I have couple of ideas, but most certainly there are many traps.

Use the correct escape sequence for reset in forth.

ryan_freqlabs.com marked an inline comment as done.Sep 26 2019, 2:56 PM