From owner-svn-src-head@freebsd.org Sat Jul 14 02:13:02 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 7EB3F10328C9 for ; Sat, 14 Jul 2018 02:13:02 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-it0-x22f.google.com (mail-it0-x22f.google.com [IPv6:2607:f8b0:4001:c0b::22f]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 0F30F83467 for ; Sat, 14 Jul 2018 02:13:02 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-it0-x22f.google.com with SMTP id l16-v6so14011990ita.0 for ; Fri, 13 Jul 2018 19:13:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20150623.gappssmtp.com; s=20150623; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=cUcTtxEiTL88q/oJUd4jf1OVUnj6pECy3MPn3XeZvE4=; b=jEziYybzujWTHsCaF/HW9AVNSNYzZ6bY32dM8mMxqLwLtDS/1S+39LFSEeW2lz0Ei9 Vm7suYbyl1wjygcnAN6uhow9tTwGgL2Bz2FqdTNqODM40YSXsNxskMFH8R4qhtxIoXq6 x50WA96jVJv1pDYhAKk/nwYtvQDV5LBd/PTRnHX7B3uY0k/qsqCXYuWtzbS2N+tU4aWH WBxc9YUNHx31awstN2eAoV7jrYH4sg4JSABeA0Bw8jNDdLgrt3Jdiqkdb4yIyMp/6lQJ YtNNAs/JxEOmVyxKTPrl5Xingrt3+GSo17YE00Kgpsq4Hs3OB2ZRlIkJlHPLitPTsDnK YUFw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=cUcTtxEiTL88q/oJUd4jf1OVUnj6pECy3MPn3XeZvE4=; b=ELNEL8+w8b8J3Obq1YP4FI04RYOZywwpDnQmRPPNe+p767j7wIcWYiN5T8Gr29r21U N4+cMJQY0JmAACtxlpqhAmrY1XRE9ruKrrT6Mf9WLZUMgBcG2F36KurRfpdpqD3pklUR K1sFJG3qMqYW6wGvBV9Ok3mc74OBoIBzkmP45c6axtD95dlM8qCu3kn9zBNqhQ2MsWVm MFrc+EemsIvo+9gshQ/PrdapBs8OT2KiJJP6mEFitfQDLNS36QDGuxkBnxATFnRCjE02 Jdwkje9qFtoBGexAoP0z/O3oFrqRCPCwkJ3UYwOrqQg3fD4sYIh/aTPvkedRg/kOMj1J B4fA== X-Gm-Message-State: AOUpUlGBPvXc6LWfdx5DQF69SNcU0G2DYsVfR2OOAlfwBqhui2rhYcI7 ckNRSH6ey+MC5BSF8Wg5KEC4TSwBX7IAF0vo8sOjMA== X-Google-Smtp-Source: AAOMgpf0IIBt1XBRBbCHWrPEXFN9FdRusOpCYkvyBOEjTkHdNZXbMuWkgcl0hZgMAGeQDKDqcW6JHp98qeBJ2fh9BTc= X-Received: by 2002:a02:3344:: with SMTP id k4-v6mr7605302jak.45.1531534381253; Fri, 13 Jul 2018 19:13:01 -0700 (PDT) MIME-Version: 1.0 Sender: wlosh@bsdimp.com Received: by 2002:a4f:1183:0:0:0:0:0 with HTTP; Fri, 13 Jul 2018 19:13:00 -0700 (PDT) X-Originating-IP: [2603:300b:6:5100:1052:acc7:f9de:2b6d] In-Reply-To: <201807140040.w6E0eddV033638@repo.freebsd.org> References: <201807140040.w6E0eddV033638@repo.freebsd.org> From: Warner Losh Date: Fri, 13 Jul 2018 20:13:00 -0600 X-Google-Sender-Auth: c4LNEd8LQcE3bsLG9RSGX4SqMNc Message-ID: Subject: Re: svn commit: r336270 - head/stand/efi/loader To: Warner Losh Cc: src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.27 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 14 Jul 2018 02:13:02 -0000 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 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); > >