Date: Thu, 30 Mar 2023 16:31:45 -0700 From: Gleb Smirnoff <glebius@freebsd.org> To: Wei Hu <whu@freebsd.org> Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org, imp@freebsd.org, gallatin@freebsd.org, kevans@freebsd.org Subject: Re: git: 927358dd98cb - main - amd64 loader: Use efiserialio for Hyper-V booted systems Message-ID: <ZCYb4V4Mg8y%2BU7tu@FreeBSD.org> In-Reply-To: <202303180720.32I7KXOc030612@gitrepo.freebsd.org> References: <202303180720.32I7KXOc030612@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Wei, Kyle, this commit hangs loader on real hardware, at least on some of it. The loader prints list of consoles and hangs hard: [Thu Mar 30 20:46:12 2023]^M|^HLoading /boot/loader.conf^M ^M/^Hconsole vidconsole is invalid!^M ^MAvailable consoles:^M ^M efi^M ^M comconsole^M ^M comconsole^M ^M nullconsole^M ^M spinconsole^M Machine is unrecoverable unless you got alternate boot media and access to BMC console. First, please DO NOT MFC this as scheduled. Second, let's try to fix it or back it out if we hear from any other CURRENT user. Our configuration isn't special. This is what we got in loader.conf, related to consoles: console="comconsole vidconsole efi" comconsole_speed=115200 comconsole_port=0x3e8 console="efi" hw.uart.console="io:1016,br:115200" On Sat, Mar 18, 2023 at 07:20:33AM +0000, Wei Hu wrote: W> The branch main has been updated by whu: W> W> URL: https://cgit.FreeBSD.org/src/commit/?id=927358dd98cb902160093e0dc0bac002d6b43858 W> W> commit 927358dd98cb902160093e0dc0bac002d6b43858 W> Author: Wei Hu <whu@FreeBSD.org> W> AuthorDate: 2023-03-14 15:13:46 +0000 W> Commit: Wei Hu <whu@FreeBSD.org> W> CommitDate: 2023-03-18 07:07:35 +0000 W> W> amd64 loader: Use efiserialio for Hyper-V booted systems W> W> UEFI provides ConIn/ConOut handles for consoles that it supports, W> which include the text-video and serial ports. When the serial port W> is available, use the UEFI driver instead of direct io-port accesses W> to avoid conflicts between the firmware and direct hardware access, as W> happens on Hyper-V (Azure) setups. W> W> This change enables efiserialio to be built for efi-amd64 and has W> higher order priority vs comconsole, and only uses efiserialio W> if the hypervisor is Hyper-V. When efiserialio successfully W> probes, it will set efi_comconsole_avail=true which will prevent W> comconsole from probing in this setup. W> W> Tested on Hyper-V, ESXi and Azure VMs. W> W> PR: 264267 W> Reviewed by: kevans, whu W> Tested by: whu W> Obtained from: Rubicon Communications, LLC (Netgate) W> MFC after: 2 weeks W> Sponsored by: Rubicon Communications, LLC (Netgate) W> --- W> stand/efi/loader/arch/amd64/Makefile.inc | 1 + W> stand/efi/loader/bootinfo.c | 11 ++++++-- W> stand/efi/loader/conf.c | 6 +++++ W> stand/efi/loader/efiserialio.c | 43 ++++++++++++++++++++++++++++---- W> stand/i386/libi386/comconsole.c | 14 +++++++++++ W> 5 files changed, 68 insertions(+), 7 deletions(-) W> W> diff --git a/stand/efi/loader/arch/amd64/Makefile.inc b/stand/efi/loader/arch/amd64/Makefile.inc W> index 0d9e2648cb59..bd89044bd6c7 100644 W> --- a/stand/efi/loader/arch/amd64/Makefile.inc W> +++ b/stand/efi/loader/arch/amd64/Makefile.inc W> @@ -5,6 +5,7 @@ SRCS+= amd64_tramp.S \ W> elf64_freebsd.c \ W> trap.c \ W> multiboot2.c \ W> + efiserialio.c \ W> exc.S W> W> .PATH: ${BOOTSRC}/i386/libi386 W> diff --git a/stand/efi/loader/bootinfo.c b/stand/efi/loader/bootinfo.c W> index 939f2cf4c3fe..d79f59343af1 100644 W> --- a/stand/efi/loader/bootinfo.c W> +++ b/stand/efi/loader/bootinfo.c W> @@ -119,10 +119,17 @@ bi_getboothowto(char *kargs) W> if (tmp != NULL) W> speed = strtol(tmp, NULL, 0); W> tmp = getenv("efi_com_port"); W> - if (tmp == NULL) W> - tmp = getenv("comconsole_port"); W> if (tmp != NULL) W> port = strtol(tmp, NULL, 0); W> + if (port <= 0) { W> + tmp = getenv("comconsole_port"); W> + if (tmp != NULL) W> + port = strtol(tmp, NULL, 0); W> + else { W> + if (port == 0) W> + port = 0x3f8; W> + } W> + } W> if (speed != -1 && port != -1) { W> snprintf(buf, sizeof(buf), "io:%d,br:%d", port, W> speed); W> diff --git a/stand/efi/loader/conf.c b/stand/efi/loader/conf.c W> index 863c9188c72c..051e1a3381d1 100644 W> --- a/stand/efi/loader/conf.c W> +++ b/stand/efi/loader/conf.c W> @@ -81,6 +81,9 @@ struct netif_driver *netif_drivers[] = { W> W> extern struct console efi_console; W> extern struct console comconsole; W> +#if defined(__amd64__) W> +extern struct console eficomconsole; W> +#endif W> #if defined(__amd64__) || defined(__i386__) W> extern struct console nullconsole; W> extern struct console spinconsole; W> @@ -88,6 +91,9 @@ extern struct console spinconsole; W> W> struct console *consoles[] = { W> &efi_console, W> +#if defined(__amd64__) W> + &eficomconsole, W> +#endif W> &comconsole, W> #if defined(__amd64__) || defined(__i386__) W> &nullconsole, W> diff --git a/stand/efi/loader/efiserialio.c b/stand/efi/loader/efiserialio.c W> index 375e679d2590..5fbc700f6ac2 100644 W> --- a/stand/efi/loader/efiserialio.c W> +++ b/stand/efi/loader/efiserialio.c W> @@ -69,6 +69,11 @@ 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> @@ -254,11 +259,22 @@ 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> + return; W> + } W> +#endif W> + W> if (comc_port == NULL) { W> comc_port = calloc(1, sizeof (struct serial)); W> if (comc_port == NULL) W> return; W> } W> + W> /* Use defaults from firmware */ W> comc_port->databits = 8; W> comc_port->parity = DefaultParity; W> @@ -308,6 +324,10 @@ comc_probe(struct console *sc) W> comc_port_set, env_nounset); W> W> env = getenv("efi_com_speed"); W> + if (env == NULL) W> + /* fallback to comconsole setting */ W> + env = getenv("comconsole_speed"); W> + W> if (comc_parse_intval(env, &val) == CMD_OK) W> comc_port->baudrate = val; W> W> @@ -318,8 +338,13 @@ comc_probe(struct console *sc) W> comc_speed_set, env_nounset); W> W> comconsole.c_flags = 0; W> - if (comc_setup()) 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> static int W> @@ -489,6 +514,7 @@ comc_setup(void) W> { W> EFI_STATUS status; W> UINT32 control; W> + char *ev; W> W> /* port is not usable */ W> if (comc_port->sio == NULL) W> @@ -498,10 +524,17 @@ comc_setup(void) W> if (EFI_ERROR(status)) W> return (false); W> 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> + 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> if (EFI_ERROR(status)) W> return (false); W> W> diff --git a/stand/i386/libi386/comconsole.c b/stand/i386/libi386/comconsole.c W> index ed1f1aa08ed7..3fbb6a292c19 100644 W> --- a/stand/i386/libi386/comconsole.c W> +++ b/stand/i386/libi386/comconsole.c W> @@ -85,6 +85,20 @@ 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?ZCYb4V4Mg8y%2BU7tu>