Page MenuHomeFreeBSD

stand: Document EFI consoles
ClosedPublic

Authored by imp on Aug 22 2022, 2:47 AM.

Details

Summary

Sponsored by: Netflix

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
This revision now requires review to proceed.Aug 22 2022, 3:38 PM

I'll also note that I'm on the record from way back saying we should not have overloaded efi to mean both text and graphics consoles. I suggested at the time we have an efigraphics and maybe default to that for the installer, but that doesn't seem to have been acted upon.

I'm also working on code so that the kernel can know the UID of the serial port efi said it was using and it will switch to it if things were improperly configured. Sadly, we can't quite do the necessary ACPI parsing before cninit (even the late one) since malloc isn't running yet. Once I do that, we can also have an efi serial thing that's more reliable (UID numbers are just a number with no correlation to anything apart from them being unique for the life of a motherboard).

But these forward-looking, hopeful things aren't yet ripe enough to document.

stand/man/loader.efi.8
103

It looks like of we use COMCONSOLE we get 9600 baud. If we use the efi console for serial, we get the default. In the 'no ConOut' set case, we'll set how = RB_SERIAL, but we won't set anything else, leaving the kernel to do default things, which will result in it using COM1 / 115200. console="comconsole" sets the speed to the default of 9600. Yes, this is confusing.

kevans added a subscriber: kevans.
kevans added inline comments.
stand/man/loader.efi.8
49
58
This revision is now accepted and ready to land.Aug 22 2022, 3:50 PM
stand/man/loader.efi.8
89

Note that this happened on a brand-new system, so it's unfortunately not a historical oddity that we can ignore and let become irrelevant over time.

I agree that we should change this documentation as we fix the related issues, but for this specific case I think this might apply to any system with no ConOut - how about "serial will be selected for the kernel boot."

Started the proposal for 115200 by default, D36295.

stand/man/loader.efi.8
267

As @manu said in the commit that added no-ConOut RB_SERIAL it seems unclear; is there some language that specifies ConOut must be provided? Ideally I could take this up with the manufacturer of the no-ConOut system, if so.

rpokala added inline comments.
stand/man/loader.efi.8
44–204

when the latter is placed in a non-UEFI filesystem.

48
.Xr efibootmgr 8 ,
53
When a system is built using
.Xr bsdinstall 8 ,
.Nm
will be installed as the default boot program,
and will therefore be used directly.
62
In
.Nm ,
this is specified by setting the
.Dv console
variable to
.Dq efi .
70

"... to all these places"?

71
.Pp
The
.Fx
kernel supports multiple consoles,
but is limited to a single
.Em primary
console.
All consoles that are configured will get kernel output;
however, only the primary console will get boot message from the
.Xr rc 8
system,
and prompts for things like
.Xr geli 8
passwords.
If
.Nm
finds a video device first, then it will be the primary console. If it finds a serial device first,
.Em that
will be the primary console.
87
variable,
the serial console will be selected.
110
.Dq efi ,
113

This entire paragraph reads... clunky, for lack of a better word. I'm not sure how to re-write it to be more clear, but it definitely needs to be re-written for clarity.

117

"... an ACPI parser"

149

Describe how the boot flags are passed to loader.efi.

151

"... by setting loader environment variables" to what value(s)?

168

This set of flags is not rendering properly. man emits:

mdoc warning: Unknown list type `Sy' (or missing list type)
              in .Bl macro

You need to replace .Bl with .It for each of the list items.

212

"... on the ESP ..."

231

"... on the ESP"

234
on the ESP.
251

"... mount the ESP, ..."

261

Need to insert

.Ed

directly before .Sh SEE ALSO, to match the .Bd from line 232.

267
variable set are not conformant with the standard,
and are likely to have unexpected results.
rpokala requested changes to this revision.Aug 22 2022, 6:07 PM
This revision now requires changes to proceed.Aug 22 2022, 6:07 PM
stand/man/loader.efi.8
53

No. It's not installed as the default boot program (except as a fallback). It's installed as efi\freebsd\loader.efi and used via efibootmgr.

89

New system or not, it is an oddity that is nonstandard. ConOut must be a proper subset of ConOutDev. They both must exist because a system must have a console. There's no conditional that states 'if they exist' or similar which makes it mandatory in my reading. I'll grant there's nothing that says that any of these variables have to exist, but I think that's splitting it a bit too fine.

However, behavior here is beyond the scope of this documentation change. It's buggy now and will be resolved in the future to a better failsafe that actually works and once the details of that are worked out, I'll document them.

113

Yup. Absent that rewrite, I'm going with what I have because it is accurate. I'll revisit when I revisit this after fixing the 'my ConOut is missing' bug that ed is having.

168

This set of flags is not rendering properly. man emits:

mdoc warning: Unknown list type `Sy' (or missing list type)
              in .Bl macro

You need to replace .Bl with .It for each of the list items.

A little too much cut and paste going on here...

267

Systems must have consoles.
ConOutDev therefore must be set because it lists the consoles
ConOut must therefore be set because its a proper (eg non-empty) subset of ConOutDev
Neither ConOut nor ConOutDev are set, therefore the system is non conforming.
See section 3.3 of at least the 2.9 standard.
There's also several items in the runtime table that are flagged as must be present if if there's no console active.
It's not 100% iron clad, since the alternative answer is that our current behavior is wrong because this indicates no console active, so we should mute all output and both our expectations are off :).

But it's likely not worth pursuing. I have this as a bug to fix, the fix will involve a behavior change and I'll document it when that happens :)

most (all?) of the suggestions have been rolled in as is, or with wording
changes to fit my (possibly overly) fussy mood today.

stand/man/loader.efi.8
267

Not worth pursuing with the BIOS vendor...

stand/man/loader.efi.8
44–204

Maybe

.Nm
when
.Nm
is placed within...

on my first read I wasn't sure if it was boot1.efi or loader.efi in the UFS or ZFS file system.

But boot1.efi is intended to be phased out anyway, so maybe make it clear that this is not the usual/desired case?
When .Nm is placed within a UFS or ZFS file system, boot1.efi is used...

82

finds a video device first in the ConOut list?

89

Right, my point is just that this documentation does not match what actually happens right now with no ConOut: the loader appears on the HDMI port while the kernel is on the serial console.

Maybe this is sufficiently described in BUGS below,

stand/man/loader.efi.8
71

Two "However"s seems clunky. I would change this first one to a paragraph break; the preceding paragraph explains console selection in loader.efi, and this new paragraph explains how that manifests in the kernel.

155

You dropped an "a", which seems necessary here.

156

"set" is unnecessary; "... to yes or no" is sufficient.

267

"... are likely"

More review comments.

stand/man/loader.efi.8
89

The documentation doesn't match what happens for you. It's not clear that the loader behaves identically on all systems that lack ConOut...
But see https://reviews.freebsd.org/D36299 for the behavior changing commit.

update with proposed 'no ConOut' behavior change currently in review.

pauamma added inline comments.
stand/man/loader.efi.8
49

Can you clarify "placed directly"?

74–75

Flows better IMO.

82–85

Could this be simplified to "... uses the first device in ConOut..."? Or are there devices types other than video and serial?

101

Audience check: will readers know what "GOP" means in this context?

stand/man/loader.efi.8
49

When it's copied to ESP:\EFI\BOOT\BOOTxxx.EFI where xxx is model specific (and technically only the default for removable devices, but almost all BIOSes will do the expected thing on fixed disks). But I'd rather not get into that in this section.

74–75

True.

82–85

Possibly. That's essentially what it does, but there's some subtly that I know this captures. One can have firewire consoles, for example, but they will never be default in UEFI.

101

Unclear. It's shorter than "a graphics capable device", but that might be a better way to say the same thing here.

267

I'll note that so far the "Exception" machines have ConIn which points to only keyboards. I suspect we can lookup each of the paths there to see what protocols it supports to make a better guess since ConIn and ConOut are to a large extent paired.

Nearly there as far as I'm concerned. Will give it a final look once content is OK.

stand/man/loader.efi.8
49

Maybe say "copied to the appropriate file in the ESP boot partition" (or "copied directly..." instead then? Leaving out the details?

101

I think that's clearer, yes.

stand/man/loader.efi.8
49

I think that this is too much to add here.

But, it's described in uefi(8). So maybe "as described in uefi(8)."

I think I've rolled all the changes in...

stand/man/loader.efi.8
74–75

You marked this "Done", but it wasn't actually done.

267

Please pick up this re-wording.

Nearly there as far as I'm concerned. Only remaining nits are the s/However/but/ rpokala mentioned you didn't actually apply, and spelling out the meaning of GOP.

stand/man/loader.efi.8
49

WFM.

pick up stragglers I somehow missed

OK. I think I have them all for real this time.

stand/man/loader.efi.8
74–75

I didn't mark this one done, phab automatically marked my comment as done, but not pau's comment which I overlooked.

Can't attest to consistency with loader.efi behavior.

This revision is now accepted and ready to land.Aug 29 2022, 5:42 AM
This revision was automatically updated to reflect the committed changes.