Sponsored by: Netflix
Details
- Reviewers
manu kevans rpokala pauamma_gundo.com - Group Reviewers
manpages - Commits
- rG8b46b5291367: stand: Document EFI consoles
rG75a91c70f8d1: stand: Document EFI consoles
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 47050 Build 43939: arc lint + arc unit
Event Timeline
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. |
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." |
stand/man/loader.efi.8 | ||
---|---|---|
44–182 | 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. | |
190 | "... on the ESP ..." | |
209 | "... on the ESP" | |
212 | on the ESP. | |
229 | "... mount the ESP, ..." | |
238 | Need to insert .Ed directly before .Sh SEE ALSO, to match the .Bd from line 232. | |
244 | variable set are not conformant with the standard, and are likely to have unexpected results. |
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 |
A little too much cut and paste going on here... | |
244 | Systems must have consoles. 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 | ||
---|---|---|
244 | Not worth pursuing with the BIOS vendor... |
stand/man/loader.efi.8 | ||
---|---|---|
44–182 | 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? | |
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. | |
244 | "... 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... |
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. | |
244 | 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. |
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)." |
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. |
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. |