Add support for custom keyboard layout when GELI asks for a passphrase

[martin_bxlr.sk]

Summary

There's a kernel build option for atkbd to have default keyboard layout specified with ATKBD_DFLT_KEYMAP. This option doesn't affect the bootloader when gptzfsboot/gptboot asks for passphrase. This patch allows users to specify this in /etc/make.conf with BOOT_DEFAULT_KEYMAP.

Test Plan

Handful of keymap layouts were tested.

Pending tests:

• what else does depend on stand/i386/common/cons.c that would cause a problem with the layout?
• how does it affect (if all) BIOSes where users have custom language already specified ?
• does my changes in Makefiles cause problems I'm not aware of?

[imp]

Some general comments...

The style of indented #if isn't what we do in FreeBSD. Code should be indented as if the #if isn't present, and the # directives should be in column 1.
We don't add extra spaces around the ('s, as I've highlighted
We don't do if (foo) bar; on one line.
logical operators are at the end of the line, not the start of the continuation.

It's an interesting notion... You'll likely want to do something similar to the EFI boot loader as well.
Other than the style issues, I think the code isn't bad... A bit worried about requiring an extra binary to build now and I'll need to think about that to make sure that we bootstrap it for the Linux and MacOS builds properly.

stand/i386/common/cons.c

117–119

FreeBSD's style would render these as

if (sf & SF_SHIFT)
        idx |= 1;
if (sf & SF_CTRL)
        idx |= 2;
if (sf & SF_ALT)
        idx |= 4;

but that likely requires a comment of some sort since it is this weird thing that kbdcontrol does to generate its maps that the reader of this code is likely unfamiliar with.

So this removes the extra spaces, and puts things on multiple lines.

122

There's an extra space after the first and third ( here.
FreeBSD style puts the || at the end of the line, not the start of the next line.

124

ditto extra spaces around ( and ).

126

extra space before sc.

128

See my general comment about indenting.

stand/i386/common/cons.h

43

The #defines here shouldn't be indented.
Extra spaces in the after ( and before ). Also extra enclosing parens around SF_LSHIFT and SF_RSHIFT

stand/i386/gptboot/Makefile

41

Bare paths like this are problematic.... this creates a race between stand and kbdcontrol potentially...

66

We generally don't include dependencies like this. Instead, consmap.h should be in the SRCS and cons.c should depend on it, not cons.o.

77

This should be done by the build system already. If not, this is likely the wrong place to put it.

Also I suspect that you'd need this in more than just gptboot, and so it should go some place common.

78

${.TARGET} here to avoid repeating the target and avoid baking in assumptions about where that file lives.

79

likewise.

stand/libsa/Makefile

193 ↗

(On Diff #103629)

This is too wide. CFLAGS += is verboten. You must only add it to the files that need it,

[tsoome]

In D34479#831843, @imp wrote:

Some general comments...

The style of indented #if isn't what we do in FreeBSD. Code should be indented as if the #if isn't present, and the # directives should be in column 1.
We don't add extra spaces around the ('s, as I've highlighted
We don't do if (foo) bar; on one line.
logical operators are at the end of the line, not the start of the continuation.

It's an interesting notion... You'll likely want to do something similar to the EFI boot loader as well.

Basically, unless we can load keyboad map via HII interface, it will not be possible, we will not get scancodes passed via SimpleTextInput(Ex) protocols.

Other than the style issues, I think the code isn't bad... A bit worried about requiring an extra binary to build now and I'll need to think about that to make sure that we bootstrap it for the Linux and MacOS builds properly.

[martin_bxlr.sk]

As this was my first patch here I need to get familiar with the interface and the way how I can submit the changes. I also need to refresh my memory what I did as I did this in March.

Other than the style issues, I think the code isn't bad... A bit worried about requiring an extra binary to build now and I'll need to think about that to make sure that we bootstrap it for the Linux and MacOS builds properly.

There's no additional binary being produced. Modification is done in getc() to use map similarly to what atkbd does.

[martin_bxlr.sk]

• I've updated the code to follow the style as you mentioned.
• I've removed too wide CFLAGS from libsa Makefile.

I was not able to solve the other Makefile issues you pointed out. This is what I've trouble with right now (my assumptions):

• Currently three binaries require cons.o: gptboot, gptzfsboot and isoboot. There's no direct rule to create cons.o in any Makefile. I'm assuming it's because it gets built by the subdir rule. I thought it would be the best to create a target cons.o that does create the mapfile if needed. As more targets require this header I thought of putting it to stand/common. I didn't find out how I can do that.

• I've included mkdir in consmap.h target. I assumed I can build stand/ directly, i.e on a fresh source copy I can do cd stand && make. The directory doesn't exist, only if I do a proper make buildkernel beforehand from src top (is this my fault of thinking about it this way?)

kbdcontrol is used in e.g. ./sys/conf/files.amd64 where the map if created for kernel option ATKBD_DFLT_KEYMAP. But I'm assuming stand/ is standalone, so it doesn't make sense to touch that config at all (same reason why I used make.conf for BOOT_DEFAULT_KEYMAP).

Side note: I know legacy boot will be used less and less. Most likely not many people require this feature at all.