Date: Fri, 31 Mar 2023 09:22:17 +0300 From: Toomas Soome <tsoome@me.com> To: Warner Losh <imp@bsdimp.com> Cc: Gleb Smirnoff <glebius@freebsd.org>, Wei Hu <whu@freebsd.org>, src-committers <src-committers@freebsd.org>, "<dev-commits-src-all@freebsd.org>" <dev-commits-src-all@freebsd.org>, "<dev-commits-src-main@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: <256C0F87-8B73-4E5E-9B5E-E9BF00DEC019@me.com> In-Reply-To: <CANCZdfpbWSbhJUDU_VUXzkqWE9Xc=MrE%2BgmEOqg%2ByOhG%2BTS6xw@mail.gmail.com> References: <202303180720.32I7KXOc030612@gitrepo.freebsd.org> <ZCYb4V4Mg8y%2BU7tu@FreeBSD.org> <CANCZdfpbWSbhJUDU_VUXzkqWE9Xc=MrE%2BgmEOqg%2ByOhG%2BTS6xw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--Apple-Mail=_CC2C5268-3B0E-412D-AB43-A76713CFE030 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 > On 31. Mar 2023, at 02:40, Warner Losh <imp@bsdimp.com> wrote: >=20 > 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 = comconsole in the first place, imho. It was OK > on arm where we couldn't conflict. >=20 > 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 = have been comconsole in the first > place because it's configured differently than comconsole.... >=20 > Warner ok, lets start from beginning. The loader device naming is total mess = and should not go this way - it is not understandable for users and not = understandable for developers. the config below is perfect example of the situation: First the config does set:=20 >> console=3D"comconsole vidconsole efi=E2=80=9D So, we do try to set serial console, bios video console and efi console. = So we have =E2=80=9Cshared=E2=80=9D config for BIOS and UEFI loader, = which should be totally reasonable from user point of view, but is = technically wrong, because BIOS loader has no idea what is efi console = and EFI loader has no idea what is vidconsole. So this mixture ends up = with message: >> console vidconsole is invalid!" And the user will get the similar error when booting BIOS setup. Then we just set console=3D=E2=80=9Cefi=E2=80=9D. Then there is setting for uart=E2=80=A6=20 With todays UEFI setups, the logic is that you set up your hardware, = that is, you configure serial port attributes from firmware setup, and = the UEFI firmware will populate its internal structures with those = values. Now, first issue is, while developing uefi loader, it was easy path to = reuse bits from BIOS loader. smbios, acpi, and sadly, comconsole. Sadly, = because comconsole is built assuming x86 IO port features which are not = available on other platforms. So yes, bios version of comconsole should not have been used to build = UEFI loader at all. Now the fun part, even as UEFI does provide SIO API to access serial = ports, to read from port, write to port and set attributes, it turns out = there are UEFI implementation(s?), where changing some attributes will = cause system to hung. Which is the reason, why we need to set up = hardware properly from firmware, and our generic config should not try = to enforce some other value(s). Default should pick up values from the = system. Sure, if the machine operator has some reason to change those = values and has tested the change, we should have method to change the = config, but again, defaults should come from system setup. While I see the reasoning to try to create all those different names, = maybe the first thing should be to look on this as innocent end user, = who has no idea about those fine technical details and historical = reasons and is just trying to get system to behave=E2=80=A6. just my 2 cents, toomas >=20 > On Thu, Mar 30, 2023 at 5:31=E2=80=AFPM Gleb Smirnoff = <glebius@freebsd.org <mailto:glebius@freebsd.org>> wrote: >> Wei, Kyle, >>=20 >> this commit hangs loader on real hardware, at least on some >> of it. The loader prints list of consoles and hangs hard: >>=20 >> [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 >>=20 >> Machine is unrecoverable unless you got alternate boot media >> and access to BMC console. >>=20 >> 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. >>=20 >> Our configuration isn't special. This is what we got in >> loader.conf, related to consoles: >>=20 >> console=3D"comconsole vidconsole efi" >> comconsole_speed=3D115200 >> comconsole_port=3D0x3e8 >> console=3D"efi" >> hw.uart.console=3D"io:1016,br:115200" >>=20 >> On Sat, Mar 18, 2023 at 07:20:33AM +0000, Wei Hu wrote: >> W> The branch main has been updated by whu: >> W>=20 >> W> URL: = https://cgit.FreeBSD.org/src/commit/?id=3D927358dd98cb902160093e0dc0bac002= d6b43858 = <https://cgit.freebsd.org/src/commit/?id=3D927358dd98cb902160093e0dc0bac00= 2d6b43858> >> W>=20 >> 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>=20 >> W> amd64 loader: Use efiserialio for Hyper-V booted systems >> W> =20 >> 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> =20 >> 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 = prevent >> W> comconsole from probing in this setup. >> W> =20 >> W> Tested on Hyper-V, ESXi and Azure VMs. >> W> =20 >> 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>=20 >> 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+=3D amd64_tramp.S \ >> W> elf64_freebsd.c \ >> W> trap.c \ >> W> multiboot2.c \ >> W> + efiserialio.c \ >> W> exc.S >> W> =20 >> 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> =20 >> 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> =20 >> 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/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> =20 >> 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> =20 >> 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> =20 >> 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> =20 >> 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> =20 >> W> @@ -318,8 +338,13 @@ comc_probe(struct console *sc) >> W> comc_speed_set, env_nounset); >> W> =20 >> 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> =20 >> W> static int >> W> @@ -489,6 +514,7 @@ comc_setup(void) >> W> { >> W> EFI_STATUS status; >> W> UINT32 control; >> W> + char *ev; >> W> =20 >> 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> =20 >> 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> =20 >> 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> =20 >> 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 =3D "xcomconsole"; >> W> + return; >> W> + } >> W> +#endif >> W> + >> W> if (comc_curspeed =3D=3D 0) { >> W> comc_curspeed =3D COMSPEED; >> W> /* >>=20 >> --=20 >> Gleb Smirnoff --Apple-Mail=_CC2C5268-3B0E-412D-AB43-A76713CFE030 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 <html><head><meta http-equiv=3D"content-type" content=3D"text/html; = charset=3Dutf-8"></head><body style=3D"overflow-wrap: break-word; = -webkit-nbsp-mode: space; line-break: = after-white-space;"><br><div><br><blockquote type=3D"cite"><div>On 31. = Mar 2023, at 02:40, Warner Losh <imp@bsdimp.com> wrote:</div><br = class=3D"Apple-interchange-newline"><div><div dir=3D"ltr">Let's back it = out. I didn't get a chance to review it and the duplicated name strikes = me as massively unwise.<div>Of course, the efi serial driver never = should have had the name comconsole in the first place, imho. It was = OK</div><div>on arm where we couldn't = conflict.</div><div><br></div><div>So let's back it out and talk about = how we should do this, including the need to possibly just rename the = efi</div><div>version of the console driver to something else. IMHO, it = never should have been comconsole in the first</div><div>place because = it's configured differently than = comconsole....</div><div><div><br></div><div>Warner</div></div></div></div= ></blockquote><div><br></div><div>ok, lets start from beginning. The = loader device naming is total mess and should not go this way - it is = not understandable for users and not understandable for = developers.</div><div><br></div><div>the config below is perfect example = of the situation:</div><div><br></div><div>First the config does = set: </div><div><br></div><div><blockquote type=3D"cite"><div = class=3D"gmail_quote"><blockquote class=3D"gmail_quote" style=3D"margin: = 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; = border-left-color: rgb(204, 204, 204); padding-left: = 1ex;">console=3D"comconsole vidconsole = efi=E2=80=9D</blockquote></div></blockquote><br></div><div>So, we do try = to set serial console, bios video console and efi console. So we have = =E2=80=9Cshared=E2=80=9D config for BIOS and UEFI loader, which should = be totally reasonable from user point of view, but is technically wrong, = because BIOS loader has no idea what is efi console and EFI loader has = no idea what is vidconsole. So this mixture ends up with = message:</div><div><br></div><blockquote type=3D"cite"><div = class=3D"gmail_quote"><blockquote class=3D"gmail_quote" style=3D"margin: = 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; = border-left-color: rgb(204, 204, 204); padding-left: 1ex;">console = vidconsole is = invalid!"</blockquote></div></blockquote><div><br></div><div>And the = user will get the similar error when booting BIOS = setup.</div><div><br></div><div>Then we just set = console=3D=E2=80=9Cefi=E2=80=9D.</div><div><br></div><div>Then there is = setting for uart=E2=80=A6 </div><div><br></div><div>With todays = UEFI setups, the logic is that you set up your hardware, that is, you = configure serial port attributes from firmware setup, and the UEFI = firmware will populate its internal structures with those = values.</div><div><br></div><div>Now, first issue is, while developing = uefi loader, it was easy path to reuse bits from BIOS loader. smbios, = acpi, and sadly, comconsole. Sadly, because comconsole is built assuming = x86 IO port features which are not available on other = platforms.</div><div><br></div><div>So yes, bios version of comconsole = should not have been used to build UEFI loader at = all.</div><div><br></div><div>Now the fun part, even as UEFI does = provide SIO API to access serial ports, to read from port, write to port = and set attributes, it turns out there are UEFI implementation(s?), = where changing some attributes will cause system to hung. Which is the = reason, why we need to set up hardware properly from firmware, and our = generic config should not try to enforce some other value(s). Default = should pick up values from the system. Sure, if the machine operator has = some reason to change those values and has tested the change, we should = have method to change the config, but again, defaults should come from = system setup.</div><div><br></div><div>While I see the reasoning to try = to create all those different names, maybe the first thing should be to = look on this as innocent end user, who has no idea about those fine = technical details and historical reasons and is just trying to get = system to behave=E2=80=A6.</div><div><br></div><div>just my 2 = cents,</div><div>toomas</div><div><br></div><blockquote = type=3D"cite"><div><br><div class=3D"gmail_quote"><div dir=3D"ltr" = class=3D"gmail_attr">On Thu, Mar 30, 2023 at 5:31=E2=80=AFPM Gleb = Smirnoff <<a = href=3D"mailto:glebius@freebsd.org">glebius@freebsd.org</a>> = wrote:<br></div><blockquote class=3D"gmail_quote" style=3D"margin:0px = 0px 0px 0.8ex;border-left:1px solid = rgb(204,204,204);padding-left:1ex"> Wei, Kyle,<br> <br> this commit hangs loader on real hardware, at least on some<br> of it. The loader prints list of consoles and hangs hard:<br> <br> [Thu Mar 30 20:46:12 2023]^M|^HLoading /boot/loader.conf^M<br> ^M/^Hconsole vidconsole is invalid!^M<br> ^MAvailable consoles:^M<br> ^M efi^M<br> ^M comconsole^M<br> ^M comconsole^M<br> ^M nullconsole^M<br> ^M spinconsole^M<br> <br> Machine is unrecoverable unless you got alternate boot media<br> and access to BMC console.<br> <br> First, please DO NOT MFC this as scheduled. Second, let's try<br> to fix it or back it out if we hear from any other CURRENT<br> user.<br> <br> Our configuration isn't special. This is what we got in<br> loader.conf, related to consoles:<br> <br> console=3D"comconsole vidconsole efi"<br> comconsole_speed=3D115200<br> comconsole_port=3D0x3e8<br> console=3D"efi"<br> hw.uart.console=3D"io:1016,br:115200"<br> <br> On Sat, Mar 18, 2023 at 07:20:33AM +0000, Wei Hu wrote:<br> W> The branch main has been updated by whu:<br> W> <br> W> URL: <a = href=3D"https://cgit.freebsd.org/src/commit/?id=3D927358dd98cb902160093e0d= c0bac002d6b43858" rel=3D"noreferrer" = target=3D"_blank">https://cgit.FreeBSD.org/src/commit/?id=3D927358dd98cb90= 2160093e0dc0bac002d6b43858</a><br> W> <br> W> commit 927358dd98cb902160093e0dc0bac002d6b43858<br> W> Author: Wei Hu <whu@FreeBSD.org><br> W> AuthorDate: 2023-03-14 15:13:46 +0000<br> W> Commit: Wei Hu <whu@FreeBSD.org><br> W> CommitDate: 2023-03-18 07:07:35 +0000<br> W> <br> W> amd64 loader: Use efiserialio for Hyper-V = booted systems<br> W> <br> W> UEFI provides ConIn/ConOut handles for consoles = that it supports,<br> W> which include the text-video and serial ports. = When the serial port<br> W> is available, use the UEFI driver instead of = direct io-port accesses<br> W> to avoid conflicts between the firmware and = direct hardware access, as<br> W> happens on Hyper-V (Azure) setups.<br> W> <br> W> This change enables efiserialio to be built for = efi-amd64 and has<br> W> higher order priority vs comconsole, and only = uses efiserialio<br> W> if the hypervisor is Hyper-V. When efiserialio = successfully<br> W> probes, it will set efi_comconsole_avail=3Dtrue = which will prevent<br> W> comconsole from probing in this setup.<br> W> <br> W> Tested on Hyper-V, ESXi and Azure VMs.<br> W> <br> W> PR: = 264267<br> W> Reviewed by: kevans, whu<br> W> Tested by: whu<br> W> Obtained from: Rubicon Communications, = LLC (Netgate)<br> W> MFC after: 2 weeks<br> W> Sponsored by: Rubicon = Communications, LLC (Netgate)<br> W> ---<br> W> stand/efi/loader/arch/amd64/Makefile.inc | 1 +<br> W> stand/efi/loader/bootinfo.c = | 11 ++++++--<br> W> stand/efi/loader/conf.c = | 6 +++++<br> W> stand/efi/loader/efiserialio.c = | 43 ++++++++++++++++++++++++++++----<br> W> stand/i386/libi386/comconsole.c = | 14 +++++++++++<br> W> 5 files changed, 68 insertions(+), 7 deletions(-)<br> W> <br> W> diff --git a/stand/efi/loader/arch/amd64/Makefile.inc = b/stand/efi/loader/arch/amd64/Makefile.inc<br> W> index 0d9e2648cb59..bd89044bd6c7 100644<br> W> --- a/stand/efi/loader/arch/amd64/Makefile.inc<br> W> +++ b/stand/efi/loader/arch/amd64/Makefile.inc<br> W> @@ -5,6 +5,7 @@ SRCS+=3D amd64_tramp.S = \<br> W> elf64_freebsd.c \<br> W> trap.c \<br> W> multiboot2.c \<br> W> + efiserialio.c \<br> W> exc.S<br> W> <br> W> .PATH: ${BOOTSRC}/i386/libi386<br> W> diff --git a/stand/efi/loader/bootinfo.c = b/stand/efi/loader/bootinfo.c<br> W> index 939f2cf4c3fe..d79f59343af1 100644<br> W> --- a/stand/efi/loader/bootinfo.c<br> W> +++ b/stand/efi/loader/bootinfo.c<br> W> @@ -119,10 +119,17 @@ bi_getboothowto(char *kargs)<br> W> = if (tmp !=3D NULL)<br> W> = speed =3D strtol(tmp, NULL, = 0);<br> W> = tmp =3D getenv("efi_com_port");<br> W> - = if (tmp =3D=3D NULL)<br> W> - = tmp =3D = getenv("comconsole_port");<br> W> = if (tmp !=3D NULL)<br> W> = port =3D strtol(tmp, NULL, = 0);<br> W> + = if (port <=3D 0) {<br> W> + = tmp =3D = getenv("comconsole_port");<br> W> + = if (tmp !=3D NULL)<br> W> + = port =3D = strtol(tmp, NULL, 0);<br> W> + = else {<br> W> + = if (port = =3D=3D 0)<br> W> + = = port =3D 0x3f8;<br> W> + = }<br> W> + = }<br> W> = if (speed !=3D -1 && port !=3D -1) {<br> W> = snprintf(buf, sizeof(buf), = "io:%d,br:%d", port,<br> W> = speed);<br> W> diff --git a/stand/efi/loader/conf.c b/stand/efi/loader/conf.c<br> W> index 863c9188c72c..051e1a3381d1 100644<br> W> --- a/stand/efi/loader/conf.c<br> W> +++ b/stand/efi/loader/conf.c<br> W> @@ -81,6 +81,9 @@ struct netif_driver *netif_drivers[] =3D {<br> W> <br> W> extern struct console efi_console;<br> W> extern struct console comconsole;<br> W> +#if defined(__amd64__)<br> W> +extern struct console eficomconsole;<br> W> +#endif<br> W> #if defined(__amd64__) || defined(__i386__)<br> W> extern struct console nullconsole;<br> W> extern struct console spinconsole;<br> W> @@ -88,6 +91,9 @@ extern struct console spinconsole;<br> W> <br> W> struct console *consoles[] =3D {<br> W> &efi_console,<br> W> +#if defined(__amd64__)<br> W> + &eficomconsole,<br> W> +#endif<br> W> &comconsole,<br> W> #if defined(__amd64__) || defined(__i386__)<br> W> &nullconsole,<br> W> diff --git a/stand/efi/loader/efiserialio.c = b/stand/efi/loader/efiserialio.c<br> W> index 375e679d2590..5fbc700f6ac2 100644<br> W> --- a/stand/efi/loader/efiserialio.c<br> W> +++ b/stand/efi/loader/efiserialio.c<br> W> @@ -69,6 +69,11 @@ static int = comc_speed_set(struct env_var *, int, const void *);<br> W> <br> W> static struct serial = *comc_port;<br> W> extern struct console efi_console;<br> W> +bool efi_comconsole_avail =3D false;<br> W> +<br> W> +#if defined(__amd64__)<br> W> +#define comconsole eficomconsole<br> W> +#endif<br> W> <br> W> struct console comconsole =3D {<br> W> .c_name =3D "comconsole",<br> W> @@ -254,11 +259,22 @@ comc_probe(struct console *sc)<br> W> char *env, *buf, *ep;<br> W> size_t sz;<br> W> <br> W> +#if defined(__amd64__)<br> W> + /*<br> W> + * For x86-64, don't use this driver if not = running in Hyper-V.<br> W> + */<br> W> + env =3D getenv("smbios.bios.version");<br> W> + if (env =3D=3D NULL || strncmp(env, "Hyper-V", 7) = !=3D 0) {<br> W> + return;<br> W> + }<br> W> +#endif<br> W> +<br> W> if (comc_port =3D=3D NULL) {<br> W> comc_port =3D = calloc(1, sizeof (struct serial));<br> W> if (comc_port =3D=3D= NULL)<br> W> = return;<br> W> }<br> W> +<br> W> /* Use defaults from firmware */<br> W> comc_port->databits =3D 8;<br> W> comc_port->parity =3D DefaultParity;<br> W> @@ -308,6 +324,10 @@ comc_probe(struct console *sc)<br> W> comc_port_set, env_nounset);<br> W> <br> W> env =3D getenv("efi_com_speed");<br> W> + if (env =3D=3D NULL)<br> W> + /* fallback to = comconsole setting */<br> W> + env =3D = getenv("comconsole_speed");<br> W> +<br> W> if (comc_parse_intval(env, &val) =3D=3D = CMD_OK)<br> W> = comc_port->baudrate =3D val;<br> W> <br> W> @@ -318,8 +338,13 @@ comc_probe(struct console *sc)<br> W> comc_speed_set, = env_nounset);<br> W> <br> W> comconsole.c_flags =3D 0;<br> W> - if (comc_setup())<br> W> + if (comc_setup()) {<br> W> sc->c_flags =3D = C_PRESENTIN | C_PRESENTOUT;<br> W> + efi_comconsole_avail =3D = true;<br> W> + } else {<br> W> + /* disable being seen = as "comconsole" */<br> W> + comconsole.c_name =3D = "efiserialio";<br> W> + }<br> W> }<br> W> <br> W> static int<br> W> @@ -489,6 +514,7 @@ comc_setup(void)<br> W> {<br> W> EFI_STATUS status;<br> W> UINT32 control;<br> W> + char *ev;<br> W> <br> W> /* port is not usable */<br> W> if (comc_port->sio =3D=3D NULL)<br> W> @@ -498,10 +524,17 @@ comc_setup(void)<br> W> if (EFI_ERROR(status))<br> W> return = (false);<br> W> <br> W> - status =3D = comc_port->sio->SetAttributes(comc_port->sio,<br> W> - comc_port->baudrate, = comc_port->receivefifodepth,<br> W> - comc_port->timeout, = comc_port->parity,<br> W> - comc_port->databits, = comc_port->stopbits);<br> W> + ev =3D getenv("smbios.bios.version");<br> W> + if (ev !=3D NULL && strncmp(ev, "Hyper-V", = 7) =3D=3D 0) {<br> W> + status =3D = comc_port->sio->SetAttributes(comc_port->sio,<br> W> + 0, 0, 0, = DefaultParity, 0, DefaultStopBits);<br> W> + } else {<br> W> + status =3D = comc_port->sio->SetAttributes(comc_port->sio,<br> W> + = comc_port->baudrate, comc_port->receivefifodepth,<br> W> + = comc_port->timeout, comc_port->parity,<br> W> + = comc_port->databits, comc_port->stopbits);<br> W> + }<br> W> +<br> W> if (EFI_ERROR(status))<br> W> return = (false);<br> W> <br> W> diff --git a/stand/i386/libi386/comconsole.c = b/stand/i386/libi386/comconsole.c<br> W> index ed1f1aa08ed7..3fbb6a292c19 100644<br> W> --- a/stand/i386/libi386/comconsole.c<br> W> +++ b/stand/i386/libi386/comconsole.c<br> W> @@ -85,6 +85,20 @@ comc_probe(struct console *cp)<br> W> int speed, port;<br> W> uint32_t locator;<br> W> <br> W> +#if defined(__amd64__)<br> W> + extern bool efi_comconsole_avail;<br> W> +<br> W> + if (efi_comconsole_avail) {<br> W> + /*<br> W> + * If EFI provides = serial I/O, then don't use this legacy<br> W> + * com driver to = avoid conflicts with the firmware's driver.<br> W> + * Change c_name = so that it cannot be found in the lookup.<br> W> + */<br> W> + comconsole.c_name =3D = "xcomconsole";<br> W> + return;<br> W> + }<br> W> +#endif<br> W> +<br> W> if (comc_curspeed =3D=3D 0) {<br> W> comc_curspeed =3D = COMSPEED;<br> W> /*<br> <br> -- <br> Gleb Smirnoff<br> </blockquote></div> </div></blockquote></div><br></body></html>= --Apple-Mail=_CC2C5268-3B0E-412D-AB43-A76713CFE030--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?256C0F87-8B73-4E5E-9B5E-E9BF00DEC019>