Date: Fri, 13 Jul 2018 20:13:00 -0600 From: Warner Losh <imp@bsdimp.com> To: Warner Losh <imp@freebsd.org> Cc: src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r336270 - head/stand/efi/loader Message-ID: <CANCZdfqpaxdKnpTD8MGaqDgt5K9b1cFTF2PsQf5d1Y3DOWonJA@mail.gmail.com> In-Reply-To: <201807140040.w6E0eddV033638@repo.freebsd.org> References: <201807140040.w6E0eddV033638@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Uggg. This commit message is slightly wrong. Added a printf I wanted to add anyway to a new version and made a note about what's wrong with it. I changed the behavior based on review feedback, but didn't change the commit message before the commit. Warner On Fri, Jul 13, 2018 at 6:40 PM, Warner Losh <imp@freebsd.org> wrote: > Author: imp > Date: Sat Jul 14 00:40:38 2018 > New Revision: 336270 > URL: https://svnweb.freebsd.org/changeset/base/336270 > > Log: > uefi stand: Guess the console better > > For server machines, ComOut is set to the set of devices that the efi > console suppots. Parse it to see if we have serial, video or both. > Make that take precidence over the command line args. boot1.efi parses > them, but loader.efi doesn't. It's not clear where to read boot.conf > from, so we don't do that. The command line args can still be set via > efibootmgr, which is more inline with the UEFI boot manager to replace > that. These args are typically used only to set serial vs video and > the com speed line. We can infer that from ComOut, so do so. > Remember the com speed and hw.uart.console to match. > > RelNotes: yes > Sponsored by: Netflix > Differential Revision: https://reviews.freebsd.org/D15917 > > Modified: > head/stand/efi/loader/bootinfo.c > head/stand/efi/loader/main.c > > Modified: head/stand/efi/loader/bootinfo.c > ============================================================ > ================== > --- head/stand/efi/loader/bootinfo.c Fri Jul 13 23:46:07 2018 > (r336269) > +++ head/stand/efi/loader/bootinfo.c Sat Jul 14 00:40:38 2018 > (r336270) > @@ -67,10 +67,11 @@ extern EFI_SYSTEM_TABLE *ST; > static int > bi_getboothowto(char *kargs) > { > - const char *sw; > + const char *sw, *tmp; > char *opts; > char *console; > - int howto; > + int howto, speed, port; > + char buf[50]; > > howto = boot_parse_cmdline(kargs); > howto |= boot_env_to_howto(); > @@ -81,6 +82,35 @@ bi_getboothowto(char *kargs) > howto |= RB_SERIAL; > if (strcmp(console, "nullconsole") == 0) > howto |= RB_MUTE; > + if (strcmp(console, "efi") == 0) { > + /* > + * If we found a com port and com speed, we need > to tell > + * the kernel where the serial port is, and how > + * fast. Ideally, we'd get the port from ACPI, but > that > + * isn't running in the loader. Do the next best > thing > + * by allowing it to be set by a loader.conf > variable, > + * either a EFI specific one, or the compatible > + * comconsole_port if not. PCI support is needed, > but > + * for that we'd ideally refactor the > + * libi386/comconsole.c code to have identical > behavior. > + */ > + tmp = getenv("efi_com_speed"); > + if (tmp != NULL) { > + speed = strtol(tmp, NULL, 0); > + tmp = getenv("efi_com_port"); > + if (tmp == NULL) > + tmp = getenv("comconsole_port"); > + /* XXX fallback to EFI variable set in > rc.d? */ > + if (tmp != NULL) > + port = strtol(tmp, NULL, 0); > + else > + port = 0x3f8; > + snprintf(buf, sizeof(buf), "io:%d,br:%d", > port, > + speed); > + env_setenv("hw.uart.console", EV_VOLATILE, > buf, > + NULL, NULL); > + } > + } > } > > return (howto); > > Modified: head/stand/efi/loader/main.c > ============================================================ > ================== > --- head/stand/efi/loader/main.c Fri Jul 13 23:46:07 2018 > (r336269) > +++ head/stand/efi/loader/main.c Sat Jul 14 00:40:38 2018 > (r336270) > @@ -400,8 +400,8 @@ interactive_interrupt(const char *msg) > return (false); > } > > -int > -parse_args(int argc, CHAR16 *argv[], bool has_kbd) > +static int > +parse_args(int argc, CHAR16 *argv[]) > { > int i, j, howto; > bool vargood; > @@ -429,12 +429,98 @@ parse_args(int argc, CHAR16 *argv[], bool has_kbd) > return (howto); > } > > +/* > + * Parse ConOut (the list of consoles active) and see if we can find a > + * serial port and/or a video port. It would be nice to also walk the > + * ACPI name space to map the UID for the serial port to a port. The > + * latter is especially hard. > + */ > +static int > +parse_uefi_con_out(void) > +{ > + int how, rv; > + int vid_seen = 0, com_seen = 0, seen = 0; > + size_t sz; > + char buf[4096], *ep; > + EFI_DEVICE_PATH *node; > + ACPI_HID_DEVICE_PATH *acpi; > + UART_DEVICE_PATH *uart; > + bool pci_pending; > > + how = 0; > + sz = sizeof(buf); > + rv = efi_global_getenv("ConOut", buf, &sz); > + if (rv != EFI_SUCCESS) > + goto out; > + ep = buf + sz; > + node = (EFI_DEVICE_PATH *)buf; > + while ((char *)node < ep && !IsDevicePathEndType(node)) { > + pci_pending = false; > + if (DevicePathType(node) == ACPI_DEVICE_PATH && > + DevicePathSubType(node) == ACPI_DP) { > + /* Check for Serial node */ > + acpi = (void *)node; > + if (EISA_ID_TO_NUM(acpi->HID) == 0x501) > + com_seen = ++seen; > + } else if (DevicePathType(node) == MESSAGING_DEVICE_PATH && > + DevicePathSubType(node) == MSG_UART_DP) { > + char bd[16]; > + > + uart = (void *)node; > + snprintf(bd, sizeof(bd), "%d", uart->BaudRate); > + setenv("efi_com_speed", bd, 1); > + printf("Console speed is %d\n", uart->BaudRate); > + } else if (DevicePathType(node) == ACPI_DEVICE_PATH && > + DevicePathSubType(node) == ACPI_ADR_DP) { > + /* Check for AcpiAdr() Node for video */ > + vid_seen = ++seen; > + } else if (DevicePathType(node) == HARDWARE_DEVICE_PATH && > + DevicePathSubType(node) == HW_PCI_DP) { > + /* > + * Note, vmware fusion has a funky console device > + * PciRoot(0x0)/Pci(0xf,0x0) > + * which we can only detect at the end since we > also > + * have to cope with: > + * PciRoot(0x0)/Pci(0x1f,0x0)/Serial(0x1) > + * so only match it if it's last. > + */ > + pci_pending = true; > + } > + node = NextDevicePathNode(node); /* Skip the end node */ > + } > + if (pci_pending && vid_seen == 0) > + vid_seen = ++seen; > + > + /* > + * Truth table for RB_MULTIPLE | RB_SERIAL > + * Value Result > + * 0 Use only video console > + * RB_SERIAL Use only serial console > + * RB_MULTIPLE Use both video and serial console > + * (but video is primary so gets rc messages) > + * both Use both video and serial console > + * (but serial is primary so gets rc messages) > + * > + * Try to honor this as best we can. If only one of serial / video > + * found, then use that. Otherwise, use the first one we found. > + * This also implies if we found nothing, default to video. > + */ > + how = 0; > + if (vid_seen && com_seen) { > + how |= RB_MULTIPLE; > + if (com_seen < vid_seen) > + how |= RB_SERIAL; > + } else if (com_seen) > + how |= RB_SERIAL; > +out: > + return (how); > +} > + > EFI_STATUS > main(int argc, CHAR16 *argv[]) > { > EFI_GUID *guid; > - int howto, i; > + int howto, i, uhowto; > UINTN k; > bool has_kbd; > char *s; > @@ -481,23 +567,61 @@ main(int argc, CHAR16 *argv[]) > */ > bcache_init(32768, 512); > > - howto = parse_args(argc, argv, has_kbd); > + howto = parse_args(argc, argv); > + if (!has_kbd && (howto & RB_PROBE)) > + howto |= RB_SERIAL | RB_MULTIPLE; > + howto &= ~RB_PROBE; > > - boot_howto_to_env(howto); > + uhowto = parse_uefi_con_out(); > > /* > - * XXX we need fallback to this stuff after looking at the ConIn, > ConOut and ConErr variables > + * We now have two notions of console. howto should be viewed as > + * overrides. > */ > - if (howto & RB_MULTIPLE) { > - if (howto & RB_SERIAL) > - setenv("console", "comconsole efi" , 1); > - else > - setenv("console", "efi comconsole" , 1); > - } else if (howto & RB_SERIAL) { > - setenv("console", "comconsole" , 1); > - } else > - setenv("console", "efi", 1); > +#define VIDEO_ONLY 0 > +#define SERIAL_ONLY RB_SERIAL > +#define VID_SER_BOTH RB_MULTIPLE > +#define SER_VID_BOTH (RB_SERIAL | RB_MULTIPLE) > +#define CON_MASK (RB_SERIAL | RB_MULTIPLE) > > + if ((howto & CON_MASK) == 0) { > + /* No override, uhowto is controlling and efi cons is > perfect */ > + howto = howto | (uhowto & CON_MASK); > + setenv("console", "efi", 1); > + } else if ((howto & CON_MASK) == (uhowto & CON_MASK)) { > + /* override matches what UEFI told us, efi console is > perfect */ > + setenv("console", "efi", 1); > + } else if ((uhowto & (CON_MASK)) != 0) { > + /* > + * We detected a serial console on ConOut. All possible > + * overrides include serial. We can't really override what > efi > + * gives us, so we use it knowing it's the best choice. > + */ > + setenv("console", "efi", 1); > + } else { > + /* > + * We detected some kind of serial in the override, but > ConOut > + * has no serial, so we have to sort out which case it > really is. > + */ > + switch (howto & CON_MASK) { > + case SERIAL_ONLY: > + setenv("console", "comconsole", 1); > + break; > + case VID_SER_BOTH: > + setenv("console", "efi comconsole", 1); > + break; > + case SER_VID_BOTH: > + setenv("console", "comconsole efi", 1); > + break; > + /* case VIDEO_ONLY can't happen -- it's the first if above > */ > + } > + } > + /* > + * howto is set now how we want to export the flags to the kernel, > so > + * set the env based on it. > + */ > + boot_howto_to_env(howto); > + > if (efi_copy_init()) { > printf("failed to allocate staging area\n"); > return (EFI_BUFFER_TOO_SMALL); > >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfqpaxdKnpTD8MGaqDgt5K9b1cFTF2PsQf5d1Y3DOWonJA>