Re: HEADS UP: broken boot! Was: git: 2f131435bc22 - main - stand: efi create eficom console device.
Date: Wed, 17 May 2023 02:19:25 UTC
On Tue, May 16, 2023, 10:12 PM Gleb Smirnoff <glebius@freebsd.org> wrote:
> Hi,
>
> this commit breaks boot on some machines with EFI. Warner already knows
> and he
> provided a quick hack to make it bootable. The proper fix will be
> available later.
>
> Meanwhile you are advised to not update your loader past 2f131435bc22, or
> use the patch (attached).
>
It breaks dual console loaders. I should have a fix in the morning. Too
much bsdcan cheer tonight to think straight.
Warner
On Thu, May 11, 2023 at 08:06:46PM +0000, Warner Losh wrote:
> W> The branch main has been updated by imp:
> W>
> W> URL:
> https://cgit.FreeBSD.org/src/commit/?id=2f131435bc22540db2d3f6bf97e5f9fe7039f889
> W>
> W> commit 2f131435bc22540db2d3f6bf97e5f9fe7039f889
> W> Author: Warner Losh <imp@FreeBSD.org>
> W> AuthorDate: 2023-05-11 20:03:17 +0000
> W> Commit: Warner Losh <imp@FreeBSD.org>
> W> CommitDate: 2023-05-11 20:06:03 +0000
> W>
> W> stand: efi create eficom console device.
> W>
> W> Fix the 'renaming kludge' that we absolutely cannot do going forward
> W> (it's cost us days of engineering time).
> W>
> W> console=comconsole talks to the hardware directly. This is available
> W> only on amd64. It is not available anywhere else (and so requires
> W> changes for people doing comconsole on aarch64)
> W>
> W> console=eficom talks to the console via EFI protocols. It's
> available
> W> on amd64, aarch64 and riscv64. It's the first port that we find,
> though
> W> it can be overriden by efi_com_port (which should be set to the UID
> of
> W> the serial port, not the I/O port, despite the name). devinfo -v
> W> will give the UID to uartX mapping.
> W>
> W> This is an incompatible change for HYPER-V on amd64. It only works
> with
> W> eficom console, so you'll need to change your configuration in
> W> loader.conf. No compatibility hack will ever be provided for this
> (since
> W> it requires renamig, which the loader cannot reliably do).
> W>
> W> It's also an incompatible change for aarch64. comconsole will need
> to
> W> change to eficom. There might be a comconsole "shim" for this.
> W>
> W> All the interlock to keep only eficom and comconsole from both
> attaching
> W> have been removed.
> W>
> W> RelNotes: Yes
> W> Sponsored by: Netflix
> W> Discussed with: kevans
> W> Differential Revision: https://reviews.freebsd.org/D39982
> W> ---
> W> stand/efi/loader/conf.c | 12 ++----
> W> stand/efi/loader/efiserialio.c | 85
> +++++++++++++++++------------------------
> W> stand/i386/libi386/comconsole.c | 14 -------
> W> 3 files changed, 39 insertions(+), 72 deletions(-)
> W>
> W> diff --git a/stand/efi/loader/conf.c b/stand/efi/loader/conf.c
> W> index 051e1a3381d1..e9ae01d19270 100644
> W> --- a/stand/efi/loader/conf.c
> W> +++ b/stand/efi/loader/conf.c
> W> @@ -80,22 +80,18 @@ struct netif_driver *netif_drivers[] = {
> W> };
> W>
> W> extern struct console efi_console;
> W> -extern struct console comconsole;
> W> -#if defined(__amd64__)
> W> -extern struct console eficomconsole;
> W> -#endif
> W> +extern struct console eficom;
> W> #if defined(__amd64__) || defined(__i386__)
> W> +extern struct console comconsole;
> W> extern struct console nullconsole;
> W> extern struct console spinconsole;
> W> #endif
> W>
> W> struct console *consoles[] = {
> W> &efi_console,
> W> -#if defined(__amd64__)
> W> - &eficomconsole,
> W> -#endif
> W> - &comconsole,
> W> + &eficom,
> W> #if defined(__amd64__) || defined(__i386__)
> W> + &comconsole,
> W> &nullconsole,
> W> &spinconsole,
> W> #endif
> W> diff --git a/stand/efi/loader/efiserialio.c
> b/stand/efi/loader/efiserialio.c
> W> index 0f37ef8b87dd..de4d6b3e34c1 100644
> W> --- a/stand/efi/loader/efiserialio.c
> W> +++ b/stand/efi/loader/efiserialio.c
> W> @@ -69,14 +69,9 @@ static int comc_speed_set(struct env_var *,
> int, const void *);
> W>
> W> static struct serial *comc_port;
> W> extern struct console efi_console;
> W> -bool efi_comconsole_avail = false;
> W>
> W> -#if defined(__amd64__)
> W> -#define comconsole eficomconsole
> W> -#endif
> W> -
> W> -struct console comconsole = {
> W> - .c_name = "comconsole",
> W> +struct console eficom = {
> W> + .c_name = "eficom",
> W> .c_desc = "serial port",
> W> .c_flags = 0,
> W> .c_probe = comc_probe,
> W> @@ -259,18 +254,6 @@ comc_probe(struct console *sc)
> W> char *env, *buf, *ep;
> W> size_t sz;
> W>
> W> -#if defined(__amd64__)
> W> - /*
> W> - * For x86-64, don't use this driver if not running in Hyper-V.
> W> - */
> W> - env = getenv("smbios.bios.version");
> W> - if (env == NULL || strncmp(env, "Hyper-V", 7) != 0) {
> W> - /* Disable being seen as "comconsole". */
> W> - comconsole.c_name = "efiserialio";
> W> - return;
> W> - }
> W> -#endif
> W> -
> W> if (comc_port == NULL) {
> W> comc_port = calloc(1, sizeof (struct serial));
> W> if (comc_port == NULL)
> W> @@ -339,13 +322,9 @@ comc_probe(struct console *sc)
> W> env_setenv("efi_com_speed", EV_VOLATILE, value,
> W> comc_speed_set, env_nounset);
> W>
> W> - comconsole.c_flags = 0;
> W> + eficom.c_flags = 0;
> W> if (comc_setup()) {
> W> sc->c_flags = C_PRESENTIN | C_PRESENTOUT;
> W> - efi_comconsole_avail = true;
> W> - } else {
> W> - /* disable being seen as "comconsole" */
> W> - comconsole.c_name = "efiserialio";
> W> }
> W> }
> W>
> W> @@ -356,7 +335,7 @@ comc_init(int arg __unused)
> W> if (comc_setup())
> W> return (CMD_OK);
> W>
> W> - comconsole.c_flags = 0;
> W> + eficom.c_flags = 0;
> W> return (CMD_ERROR);
> W> }
> W>
> W> @@ -522,35 +501,41 @@ comc_setup(void)
> W> if (comc_port->sio == NULL)
> W> return (false);
> W>
> W> - status = comc_port->sio->Reset(comc_port->sio);
> W> - if (EFI_ERROR(status))
> W> - return (false);
> W> -
> W> - ev = getenv("smbios.bios.version");
> W> - if (ev != NULL && strncmp(ev, "Hyper-V", 7) == 0) {
> W> - status = comc_port->sio->SetAttributes(comc_port->sio,
> W> - 0, 0, 0, DefaultParity, 0, DefaultStopBits);
> W> - } else {
> W> - status = comc_port->sio->SetAttributes(comc_port->sio,
> W> - comc_port->baudrate, comc_port->receivefifodepth,
> W> - comc_port->timeout, comc_port->parity,
> W> - comc_port->databits, comc_port->stopbits);
> W> + if (comc_port->sio->Reset != NULL) {
> W> + status = comc_port->sio->Reset(comc_port->sio);
> W> + if (EFI_ERROR(status))
> W> + return (false);
> W> }
> W>
> W> - if (EFI_ERROR(status))
> W> - return (false);
> W> + if (comc_port->sio->SetAttributes != NULL) {
> W> + ev = getenv("smbios.bios.version");
> W> + if (ev != NULL && strncmp(ev, "Hyper-V", 7) == 0) {
> W> + status =
> comc_port->sio->SetAttributes(comc_port->sio,
> W> + 0, 0, 0, DefaultParity, 0, DefaultStopBits);
> W> + } else {
> W> + status =
> comc_port->sio->SetAttributes(comc_port->sio,
> W> + comc_port->baudrate,
> comc_port->receivefifodepth,
> W> + comc_port->timeout, comc_port->parity,
> W> + comc_port->databits, comc_port->stopbits);
> W> + }
> W>
> W> - status = comc_port->sio->GetControl(comc_port->sio, &control);
> W> - if (EFI_ERROR(status))
> W> - return (false);
> W> - if (comc_port->rtsdtr_off) {
> W> - control &= ~(EFI_SERIAL_REQUEST_TO_SEND |
> W> - EFI_SERIAL_DATA_TERMINAL_READY);
> W> - } else {
> W> - control |= EFI_SERIAL_REQUEST_TO_SEND;
> W> + if (EFI_ERROR(status))
> W> + return (false);
> W> + }
> W> +
> W> + if (comc_port->sio->GetControl != NULL &&
> comc_port->sio->SetControl != NULL) {
> W> + status = comc_port->sio->GetControl(comc_port->sio,
> &control);
> W> + if (EFI_ERROR(status))
> W> + return (false);
> W> + if (comc_port->rtsdtr_off) {
> W> + control &= ~(EFI_SERIAL_REQUEST_TO_SEND |
> W> + EFI_SERIAL_DATA_TERMINAL_READY);
> W> + } else {
> W> + control |= EFI_SERIAL_REQUEST_TO_SEND;
> W> + }
> W> + (void) comc_port->sio->SetControl(comc_port->sio, control);
> W> }
> W> - (void) comc_port->sio->SetControl(comc_port->sio, control);
> W> /* Mark this port usable. */
> W> - comconsole.c_flags |= (C_PRESENTIN | C_PRESENTOUT);
> W> + eficom.c_flags |= (C_PRESENTIN | C_PRESENTOUT);
> W> return (true);
> W> }
> W> diff --git a/stand/i386/libi386/comconsole.c
> b/stand/i386/libi386/comconsole.c
> W> index 507cd0ec922f..6d48e876fa37 100644
> W> --- a/stand/i386/libi386/comconsole.c
> W> +++ b/stand/i386/libi386/comconsole.c
> W> @@ -85,20 +85,6 @@ comc_probe(struct console *cp)
> W> int speed, port;
> W> uint32_t locator;
> W>
> W> -#if defined(__amd64__)
> W> - extern bool efi_comconsole_avail;
> W> -
> W> - if (efi_comconsole_avail) {
> W> - /*
> W> - * If EFI provides serial I/O, then don't use this legacy
> W> - * com driver to avoid conflicts with the firmware's
> driver.
> W> - * Change c_name so that it cannot be found in the lookup.
> W> - */
> W> - comconsole.c_name = "xcomconsole";
> W> - return;
> W> - }
> W> -#endif
> W> -
> W> if (comc_curspeed == 0) {
> W> comc_curspeed = COMSPEED;
> W> /*
>
> --
> Gleb Smirnoff
>