loader: fix EFI ACPI detection

[rcm]

Summary

lua was previously unable to determine ACPI presence because this
probing was postponed until the final loading and execution of the
kernel.

This patch resolves that by detecting ACPI early (similar to
the order of operations in the legacy i386 loader).

Sponsored by: Rubicon Communications, LLC ("Netgate")

[imp]

I'll take a look at the rest of this later...

stand/efi/loader/main.c

917

This brings back old, deprecated hints that we've specifically removed from x86.
We likely can walk away from having them on arm at this point.

[rcm]

stand/efi/loader/main.c

917

Yes I noticed your commits elsewhere that removed these hints. I can follow suit and align.

[rcm]

Address Warner's comment regarding legacy hints and update the man page as suggested by Kyle

[imp]

I generally like this change, one or two quibbles to work out.
I'd also split the lua and other stuff into separate commits (the old lua code will work with the new loader.efi, which I like as well).
But I can do the splitting if that's a hassle. This is otherwise fairly clean so I wouldn't mind a small amount of extra work.

stand/efi/loader/main.c

985

does this work for armv7 and riscv who also use this loader?

stand/lua/core.lua.8

71 ↗

(On Diff #129696)

Why this change?

93 ↗

(On Diff #129696)

this should be SetSingleUser, since that's now the longest one.

[rcm]

Stripped out lua diff to submit separately

[rcm]

In D42459#969394, @imp wrote:

I generally like this change, one or two quibbles to work out.
I'd also split the lua and other stuff into separate commits (the old lua code will work with the new loader.efi, which I like as well).
But I can do the splitting if that's a hassle. This is otherwise fairly clean so I wouldn't mind a small amount of extra work.

Yep. I have stripped out the lua diff and will get that posted separately in a moment.

[rcm]

stand/efi/loader/main.c

985

Good question. Maybe we should ifdef out armv7 and risc-v for now? I'm new to this level of work, but from what I can tell this won't ever work on armv7 and could work on risc-v (the Linux kernel just got basic ACPI infrastructure wired up in March of this year). So, I have no idea where that stands on FreeBSD at the moment.

[rcm]

Added @jrtc27 and @mhorne to chime in concerning @imp's question concerning risc-v (and possibly armv7)

[mhorne]

stand/efi/loader/main.c

985

Yeah there is no active or planned work for ACPI support for riscv, but we should move forward with the assumption that it will come eventually.

If the conditional could be expressed as a feature flag, e.g. #if _EFI_LOADER_HAVE_ACPI == 1, that might be nice, but IMO an #ifndef __riscv is also fine.

[jrtc27]

stand/efi/loader/main.c

985

If the code is MI and compiles just fine on RISC-V (but always early-exits as "no ACPI found") then there's no need to #ifdef it though.

And yeah, ACPI for RISC-V won't happen until the specs settle down and people actually use it.

[rcm]

stand/efi/loader/main.c

985

A quick glance at libefi suggests that this should compile fine as-is and acpi_detect would just return early.

[kevans]

I'm not seeing any remaining problems here, seems to LGTM.

[imp]

This broke the armv7 build...
https://ci.freebsd.org/job/FreeBSD-main-armv7-build/19710/
Please fix ASAP.

stand/efi/loader/main.c

985

As long as it works (in that it fails to detect it and returns false), then we don't need / want an ifdef.

[imp]

src/sys/contrib/dev/acpica/include/actypes.h:235:41: error: redefinition of typedef 'BOOLEAN' is a C11 feature [-Werror,-Wtypedef-redefinition]
typedef unsigned char BOOLEAN;

^

/usr/src/stand/efi/include/efidef.h:33:25: note: previous definition is here
typedef UINT8 BOOLEAN;