From nobody Thu Mar 30 23:45:19 2023 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4Png5l21qrz42Xmc; Thu, 30 Mar 2023 23:45:31 +0000 (UTC) (envelope-from kevans@freebsd.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Png5k6bLJz4Khp; Thu, 30 Mar 2023 23:45:30 +0000 (UTC) (envelope-from kevans@freebsd.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1680219930; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=MFmHFjoPKzIMuLMSwK8jzfTrHjhzjM8nW3QYxyMO9v8=; b=KE+fhFxEezaGRtZFKaRt/kvk9RSATNMGX6wsLo9v6gFriaGnoW6I3sgjZUsr7i4X4mHAMB Ln2EzAAZ4NuBFwJO/s/Ilt7jDi9kNck7KKP4Xrw6jtQK6kI79hM44ObNMCYBVMHkG2wotJ qys7GR/Qz7hTfRq5hwjcA128f6shNK8K9N+Y+xNbfuXjHU6L3aw2OAYJe2cOXTm8gr0DrQ liXEgirra/kwIC4VVGYVVFAnoaK3MMgkdfdtRVZMIrXtJEwUOWgD8DYwhNilmxpq7XjzXy /RRbDyiGxlO/It1S0j4vz2PF2moxnZFpMpTG/WP0K/tpcOMMui0hY51eHJKt5Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1680219930; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=MFmHFjoPKzIMuLMSwK8jzfTrHjhzjM8nW3QYxyMO9v8=; b=htTXNCnP7RMApFiWEMk7gC2JqCOaU6yKrAudSQv0HOJjKVRbE2VxrbDurv2ioD18YlC1X3 cTPH1WAxnPJCcj1/npaNejgRgB+3b9ZjSJZfgWEhOpRByhahvOjPI0E6EsaM6P6RfDZMNj 0+5tO+VzzmvkdL3iCB7k/oyCLC/ar3cPkCx1Je60PKI762HFzwU7PaAAiw+VotNgdMu9pk P4TNIY0XwK5JNcNRMckdcbvyEruzwwFLPekTtEdvy4Jz2GmpnNOpdXxvpYKKrdAg/EeydK OGEk9RqBHQp/bKwrUMYO5pFEs96sFv2cxygXyRMH07OzeTCiSq5uW1gMZHxwIA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1680219930; a=rsa-sha256; cv=none; b=RBqzIZbM8etzFWVPHElYHCfcZ3PCOlpX/NiOfLa7fHipLxkafDamp63ztYL+qQQq2aslNK ZRnsMVAgiPCiGGCHOs5Ffw5zq1Ne1QH5csXrl5AizQVLsMr772y3RU05/rVF5a5kOaGhR5 eH06lNZ2nZco5MOETdItdgZH3yqLz61jg2q6B/KuZ48yFGK6e4kzqBI1Smzok5F9U2FDrq mxq9F1V+bpa58zgkGe6G7RWDnVjxrdfrFP+MecqyCDqs5Ap6JuLvPfOtiU5gznQiCkLgO7 7Q9rP5J1bilqrqRLE10lWT4UxY0oJG4bVNn9nrPQMbc3FQLA/goUORh2mKfq3Q== Received: from mail-qt1-f171.google.com (mail-qt1-f171.google.com [209.85.160.171]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) (Authenticated sender: kevans) by smtp.freebsd.org (Postfix) with ESMTPSA id 4Png5k5WwSzmwP; Thu, 30 Mar 2023 23:45:30 +0000 (UTC) (envelope-from kevans@freebsd.org) Received: by mail-qt1-f171.google.com with SMTP id a5so20189680qto.6; Thu, 30 Mar 2023 16:45:30 -0700 (PDT) X-Gm-Message-State: AAQBX9d+FyQmf9RLSWTLHBZI4H0buX3kN+0Z0xuWZe8fG+W2gDusdk5D PvcbC087QJt1EUuhcUZajKeBP5Iw54C9hZh/mkw= X-Google-Smtp-Source: AKy350aUB6x6AsyuongUGbPpszXXrTneWaTVO4UzgSA+yTSKQ35p/XdY+TGFxVl5HP8pzinTWa3wKOQFoJTD1FySA4w= X-Received: by 2002:ac8:59c8:0:b0:3e4:de7a:d532 with SMTP id f8-20020ac859c8000000b003e4de7ad532mr7756042qtf.13.1680219930180; Thu, 30 Mar 2023 16:45:30 -0700 (PDT) List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 References: <202303180720.32I7KXOc030612@gitrepo.freebsd.org> In-Reply-To: From: Kyle Evans Date: Thu, 30 Mar 2023 18:45:19 -0500 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: git: 927358dd98cb - main - amd64 loader: Use efiserialio for Hyper-V booted systems To: Warner Losh Cc: Gleb Smirnoff , Wei Hu , 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 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-ThisMailContainsUnwantedMimeParts: N On Thu, Mar 30, 2023 at 6:40=E2=80=AFPM Warner Losh wrote: > > Let's back it out. I didn't get a chance to review it and the duplicated = name strikes me as massively unwise. > Of course, the efi serial driver never should have had the name comconsol= e in the first place, imho. It was OK > on arm where we couldn't conflict. > > So let's back it out and talk about how we should do this, including the = need to possibly just rename the efi > version of the console driver to something else. IMHO, it never should ha= ve been comconsole in the first > place because it's configured differently than comconsole.... > Sure, no objection. > Warner > > On Thu, Mar 30, 2023 at 5:31=E2=80=AFPM Gleb Smirnoff wrote: >> >> 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=3D"comconsole vidconsole efi" >> comconsole_speed=3D115200 >> comconsole_port=3D0x3e8 >> console=3D"efi" >> hw.uart.console=3D"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=3D927358dd98cb902160093e= 0dc0bac002d6b43858 >> W> >> W> commit 927358dd98cb902160093e0dc0bac002d6b43858 >> W> Author: Wei Hu >> W> AuthorDate: 2023-03-14 15:13:46 +0000 >> W> Commit: Wei Hu >> 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 po= rt >> W> is available, use the UEFI driver instead of direct io-port acces= ses >> W> to avoid conflicts between the firmware and direct hardware acces= s, 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=3Dtrue which will preven= t >> 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/loa= der/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+=3D 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 !=3D NULL) >> W> speed =3D strtol(tmp, NULL, 0); >> W> tmp =3D getenv("efi_com_port"); >> W> - if (tmp =3D=3D NULL) >> W> - tmp =3D getenv("comconsole_port"); >> W> if (tmp !=3D NULL) >> W> port =3D strtol(tmp, NULL, 0); >> W> + if (port <=3D 0) { >> W> + tmp =3D getenv("comconsole_port"); >> W> + if (tmp !=3D NULL) >> W> + port =3D strtol(tmp, NULL, 0); >> W> + else { >> W> + if (port =3D=3D 0) >> W> + port =3D 0x3f8; >> W> + } >> W> + } >> W> if (speed !=3D -1 && port !=3D -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[] =3D { >> 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[] =3D { >> 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/efiser= ialio.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 =3D false; >> W> + >> W> +#if defined(__amd64__) >> W> +#define comconsole eficomconsole >> W> +#endif >> W> >> W> struct console comconsole =3D { >> W> .c_name =3D "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 =3D getenv("smbios.bios.version"); >> W> + if (env =3D=3D NULL || strncmp(env, "Hyper-V", 7) !=3D 0) { >> W> + return; >> W> + } >> W> +#endif >> W> + >> W> if (comc_port =3D=3D NULL) { >> W> comc_port =3D calloc(1, sizeof (struct serial)); >> W> if (comc_port =3D=3D NULL) >> W> return; >> W> } >> W> + >> W> /* Use defaults from firmware */ >> W> comc_port->databits =3D 8; >> W> comc_port->parity =3D DefaultParity; >> W> @@ -308,6 +324,10 @@ comc_probe(struct console *sc) >> W> comc_port_set, env_nounset); >> W> >> W> env =3D getenv("efi_com_speed"); >> W> + if (env =3D=3D NULL) >> W> + /* fallback to comconsole setting */ >> W> + env =3D getenv("comconsole_speed"); >> W> + >> W> if (comc_parse_intval(env, &val) =3D=3D CMD_OK) >> W> comc_port->baudrate =3D val; >> W> >> W> @@ -318,8 +338,13 @@ comc_probe(struct console *sc) >> W> comc_speed_set, env_nounset); >> W> >> W> comconsole.c_flags =3D 0; >> W> - if (comc_setup()) >> W> + if (comc_setup()) { >> W> sc->c_flags =3D C_PRESENTIN | C_PRESENTOUT; >> W> + efi_comconsole_avail =3D true; >> W> + } else { >> W> + /* disable being seen as "comconsole" */ >> W> + comconsole.c_name =3D "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 =3D=3D NULL) >> W> @@ -498,10 +524,17 @@ comc_setup(void) >> W> if (EFI_ERROR(status)) >> W> return (false); >> W> >> W> - status =3D 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 =3D getenv("smbios.bios.version"); >> W> + if (ev !=3D NULL && strncmp(ev, "Hyper-V", 7) =3D=3D 0) { >> W> + status =3D comc_port->sio->SetAttributes(comc_port->sio, >> W> + 0, 0, 0, DefaultParity, 0, DefaultStopBits); >> W> + } else { >> W> + status =3D 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/com= console.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 legac= y >> W> + * com driver to avoid conflicts with the firmware's dri= ver. >> W> + * Change c_name so that it cannot be found in the looku= p. >> W> + */ >> W> + comconsole.c_name =3D "xcomconsole"; >> W> + return; >> W> + } >> W> +#endif >> W> + >> W> if (comc_curspeed =3D=3D 0) { >> W> comc_curspeed =3D COMSPEED; >> W> /* >> >> -- >> Gleb Smirnoff