add beastie support for loader.efi
ClosedPublic
Actions

Authored by tsoome on Jan 5 2016, 9:53 AM.

Details

Reviewers

emaste
adrian

Group Reviewers

Contributor Reviewers (ports)

Commits

rS293233: loader.efi: add terminal emulation support

Summary

This is "backport" of the work I did at illumos side and is essentially based on vidconsole implementation.

few notes:
the getchar() needs to translate special keys, I did left in the switch (in illumos code I was also translating arrow keys etc) even as it currently has only entry for esc key.

"box" bios codes are translated for putchar. the issue is, in EFI the character codes are multibyte and as ficl does not support multibyte chars, this translation is currently most reasonable. There is an catch tho - as in bios the box chars are upper half of the table and therefore are having "negative" values, this will bring in the signed versus unsigned char issue for putchar(). In this diff I did add cast to unsigned char for amd64 variant of ficl. This allows to use meaningful hex codes for bios box chars in putchar.

And final note is about HO() call added for mode command - to make mode switches more "clean".

Test Plan

UI is functional, ESC sequences do work.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

tsoome updated this revision to Diff 11944.Jan 5 2016, 9:53 AM

tsoome retitled this revision from to add beastie support for loader.efi.

tsoome updated this object.

tsoome edited the test plan for this revision. (Show Details)

tsoome set the repository for this revision to rS FreeBSD src repository - subversion.

Herald added a reviewer: Contributor Reviewers (ports). · View Herald TranscriptJan 5 2016, 9:53 AM

Herald added a subscriber: imp. · View Herald Transcript

allanjude added a subscriber: allanjude.Jan 5 2016, 4:42 PM

Thank you for this work! I'll review and test as soon as I can.

emaste added inline comments.Jan 5 2016, 6:33 PM

sys/boot/efi/libefi/Makefile
24	Is there a reason an end user wouldn't want `TERM_EMU` supported (at build time)?
sys/boot/efi/libefi/efi_console.c
2–3	Perhaps this should carry an additional copyright
72–74	Minor nit, different comment style here compared to other functions
133	Should have an XXX marker for now (and be fixed in the future)

f0andrey_gmail.com added a subscriber: f0andrey_gmail.com.Jan 5 2016, 6:38 PM

I have been wondering the same really, but I left it in to have same semantics with bios version (vidconsole). I think it should get removed eventually, especially as without terminal support, the beastie will be useless anyhow. I dont know about freebsd traditions about such updates - I personally would rather just see followup issue created to remove TERM_EMU:)

In D4797#101848, @tsoome_me.com wrote:

I have been wondering the same really, but I left it in to have same semantics with bios version (vidconsole). I think it should get removed eventually, especially as without terminal support, the beastie will be useless anyhow. I dont know about freebsd traditions about such updates - I personally would rather just see followup issue created to remove TERM_EMU:)

I know some places have used it to reduce the footprint of the resulting binary, at least for vidconsole. It makes enough of a difference that it is useful there. However, the target market for UEFI I'm less sure of. The only place I think it might matter is for the embedded i386 kit that boots with 32-bit EFI, but those should be beefy enough that I doubt this would matter.

There's a lot of people that don't care about beastie support and compile it out / disable it because it is useless bloat for a boot loader, even if the space savings isn't a consideration.

I'm not sure how justified it would be in sense that this update is really just duplication of vidconsole. Perhaps later when more proper terminal io support will be added, then it would be more appropriate. In illumos version I actually have cli editor with history as well, but I want to keep this update small and self contained, also the linenoise (used for cli editor), is a bit too "dumb" to support serial console properly and that option would need more investigation.

will added a subscriber: will.Jan 5 2016, 6:57 PM

The only place I think it might matter is for the embedded i386 kit that boots with 32-bit EFI, but those should be beefy enough that I doubt this would matter.

There's a lot of people that don't care about beastie support and compile it out / disable it because it is useless bloat for a boot loader, even if the space savings isn't a consideration.

Yeah, the extra couple of kbytes is not going to matter for any system with UEFI firmware. I also turn the beastie support off on my own systems, but don't see a need for a build time option here to save the bit of extra space.

I'd have a slight worry that the !TERM_EMU case will bitrot, but in any case it already exists so I'd just commit it as is.

updated comments as requested.

fixed

In D4797#101857, @imp wrote:

In D4797#101848, @tsoome_me.com wrote:

I have been wondering the same really, but I left it in to have same semantics with bios version (vidconsole). I think it should get removed eventually, especially as without terminal support, the beastie will be useless anyhow. I dont know about freebsd traditions about such updates - I personally would rather just see followup issue created to remove TERM_EMU:)

I know some places have used it to reduce the footprint of the resulting binary, at least for vidconsole. It makes enough of a difference that it is useful there. However, the target market for UEFI I'm less sure of. The only place I think it might matter is for the embedded i386 kit that boots with 32-bit EFI, but those should be beefy enough that I doubt this would matter.

There's a lot of people that don't care about beastie support and compile it out / disable it because it is useless bloat for a boot loader, even if the space savings isn't a consideration.

of course it is not hard to remove it now, but having it around for now also adds some possible value (even if slim). I personally like the view where options for different platforms are similar. But if people feel it is really unnecessary, I don't really mind to nuke it.